Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .changeset/plugin-storage-range-filter-guard.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
"emdash": patch
---

Fixes a plugin storage range filter whose every bound is `undefined` matching every row instead of failing. Building a bound from an optional value — `where: { stock: { gte: minStock } }` where `minStock` is `undefined` — type-checks, but contributed no SQL, so `query()` and `count()` returned the whole collection and `updateIf()` applied its write with no guard at all. A guarded decrement could then drive a counter past the bound the caller asked for.

Such a filter now throws `StorageQueryError` naming the field. Pass a defined bound, or omit the field when you mean to match unconditionally:

```typescript
const where = minStock === undefined ? {} : { stock: { gte: minStock } };
await ctx.storage.products.query({ where });
```
13 changes: 13 additions & 0 deletions packages/core/src/plugins/storage-query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,17 @@ export function buildCondition(
if (value.lt !== undefined) pushBound("<", value.lt);
if (value.lte !== undefined) pushBound("<=", value.lte);

// A filter with no defined bound contributes no SQL. Returning it would
// widen the caller's predicate to "match everything" — survivable in a
// read, but it strips the guard off a conditional write.
if (conditions.length === 0) {
throw new StorageQueryError(
`Range filter for field '${field}' has no defined bound`,
field,
"Provide at least one of gt, gte, lt, or lte, or omit the field.",
);
}

return {
sql: conditions.join(" AND "),
params,
Expand All @@ -268,6 +279,8 @@ export function buildWhereClause(

for (const [field, value] of Object.entries(where)) {
const condition = buildCondition(db, field, value);
// An empty slot in the join would emit `<cond> AND ` and fail to parse.
if (!condition.sql) continue;
conditions.push(condition.sql);
params.push(...condition.params);
}
Expand Down
15 changes: 15 additions & 0 deletions packages/core/tests/integration/plugins/storage-updateif.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { it, expect, beforeEach, afterEach } from "vitest";

import { PluginStorageRepository } from "../../../src/database/repositories/plugin-storage.js";
import type { Database } from "../../../src/database/types.js";
import { StorageQueryError } from "../../../src/plugins/storage-query.js";
import {
describeEachDialect,
setupForDialect,
Expand Down Expand Up @@ -191,6 +192,20 @@ describeEachDialect("Plugin storage updateIf", (dialect) => {
expect((await repo.get("p1"))?.stock).toBe(5);
});

it("updateIf() with an all-`undefined` guard throws instead of writing unguarded", async () => {
// An empty predicate contributes no SQL. Dropped silently, it would leave
// the UPDATE unguarded and drive stock past the bound the caller asked for.
const repo = productsRepo();
await repo.put("p1", { sku: "A", stock: 0, tier: 1, name: "Alpha" });
await expect(
repo.updateIf("p1", {
where: { stock: { gte: undefined } },
delta: { stock: { dec: 1 } },
}),
).rejects.toThrow(StorageQueryError);
expect((await repo.get("p1"))?.stock).toBe(0);
});

it("updateIf() with an all-`undefined` set (no delta) throws and never writes", async () => {
const repo = productsRepo();
await repo.put("p1", { sku: "A", stock: 5, tier: 1, name: "Alpha" });
Expand Down
18 changes: 18 additions & 0 deletions packages/core/tests/integration/plugins/storage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,24 @@ describe("Plugin Storage Integration", () => {
const plain = await repo.query({ where: { eventType: { startsWith: "sale:" } } });
expect(plain.items).toHaveLength(2);
});

it("rejects a range filter with no defined bound instead of matching every row", async () => {
// A bound built from an optional value is the common way to reach this;
// dropping the predicate would silently widen the query to the whole
// collection.
const repo = new PluginStorageRepository<AnalyticsEvent>(db, "analytics", "events", [
"timestamp",
]);
await repo.putMany([
{ id: "e1", data: { eventType: "a", userId: "u1", timestamp: "2024-01-01", metadata: {} } },
{ id: "e2", data: { eventType: "b", userId: "u2", timestamp: "2024-06-01", metadata: {} } },
]);

const since: string | undefined = undefined;
await expect(repo.query({ where: { timestamp: { gte: since } } })).rejects.toThrow(
/no defined bound/,
);
});
});

describe("createPluginStorageAccessor", () => {
Expand Down
17 changes: 17 additions & 0 deletions packages/core/tests/unit/plugins/storage-query.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,15 @@ describe("storage-query", () => {
expect(result.sql).toBe(`${ageNum} >= ? AND ${ageNum} < ?`);
expect(result.params).toEqual([18, 65]);
});

it("should throw for a range filter whose every bound is undefined", () => {
// An empty predicate is silently dropped downstream, which turns a
// guard into an unconditional match.
expect(() => buildCondition(db, "age", { gte: undefined })).toThrow(StorageQueryError);
expect(() => buildCondition(db, "age", { gt: undefined, lte: undefined })).toThrow(
StorageQueryError,
);
});
});

describe("buildWhereClause", () => {
Expand Down Expand Up @@ -314,6 +323,14 @@ describe("storage-query", () => {
expect(result.sql).toContain(">= ?");
expect(result.params).toEqual(["active", "pending", "test%", 5]);
});

it("should never emit a dangling AND", () => {
// A condition that contributes no SQL must not leave an empty slot in
// the join, which would produce `<cond> AND ` and fail to parse.
expect(() => buildWhereClause(db, { count: { gte: -99 }, name: { gte: undefined } })).toThrow(
StorageQueryError,
);
});
});

describe("buildOrderByClause", () => {
Expand Down
3 changes: 2 additions & 1 deletion skills/creating-plugins/references/storage.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ const result = await ctx.storage.submissions.query({
// Exact match
where: { status: "pending" }

// Range
// Range — needs at least one defined bound, or it throws StorageQueryError
where: { createdAt: { gte: "2024-01-01" } }
where: { score: { gt: 50, lte: 100 } }

Expand Down Expand Up @@ -155,6 +155,7 @@ Behavior to account for:
- `delta` accepts integers. A float throws `TypeError`.
- Pass at least one of `set` or `delta`. A field cannot appear in both.
- A `dec` drives a field negative when the guard does not cover it. Pair `dec: k` with a `gte: k` guard to keep the field at or above zero.
- A range filter in the guard needs at least one defined bound. `{ stock: { gte: undefined } }` throws `StorageQueryError` rather than leaving the write unguarded.
- A losing writer that aborts instead of returning `{ applied: false }` throws `StorageSerializationError`, carrying the Postgres SQLSTATE (`40001` or `40P01`). Retry the call.

### Index Design
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ const result = await ctx.storage.submissions.query({
// Exact match
where: { status: "pending" }

// Range
// Range — needs at least one defined bound, or it throws StorageQueryError
where: { createdAt: { gte: "2024-01-01" } }
where: { score: { gt: 50, lte: 100 } }

Expand Down Expand Up @@ -155,6 +155,7 @@ Behavior to account for:
- `delta` accepts integers. A float throws `TypeError`.
- Pass at least one of `set` or `delta`. A field cannot appear in both.
- A `dec` drives a field negative when the guard does not cover it. Pair `dec: k` with a `gte: k` guard to keep the field at or above zero.
- A range filter in the guard needs at least one defined bound. `{ stock: { gte: undefined } }` throws `StorageQueryError` rather than leaving the write unguarded.
- A losing writer that aborts instead of returning `{ applied: false }` throws `StorageSerializationError`, carrying the Postgres SQLSTATE (`40001` or `40P01`). Retry the call.

### Index Design
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ const result = await ctx.storage.submissions.query({
// Exact match
where: { status: "pending" }

// Range
// Range — needs at least one defined bound, or it throws StorageQueryError
where: { createdAt: { gte: "2024-01-01" } }
where: { score: { gt: 50, lte: 100 } }

Expand Down Expand Up @@ -155,6 +155,7 @@ Behavior to account for:
- `delta` accepts integers. A float throws `TypeError`.
- Pass at least one of `set` or `delta`. A field cannot appear in both.
- A `dec` drives a field negative when the guard does not cover it. Pair `dec: k` with a `gte: k` guard to keep the field at or above zero.
- A range filter in the guard needs at least one defined bound. `{ stock: { gte: undefined } }` throws `StorageQueryError` rather than leaving the write unguarded.
- A losing writer that aborts instead of returning `{ applied: false }` throws `StorageSerializationError`, carrying the Postgres SQLSTATE (`40001` or `40P01`). Retry the call.

### Index Design
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ const result = await ctx.storage.submissions.query({
// Exact match
where: { status: "pending" }

// Range
// Range — needs at least one defined bound, or it throws StorageQueryError
where: { createdAt: { gte: "2024-01-01" } }
where: { score: { gt: 50, lte: 100 } }

Expand Down Expand Up @@ -155,6 +155,7 @@ Behavior to account for:
- `delta` accepts integers. A float throws `TypeError`.
- Pass at least one of `set` or `delta`. A field cannot appear in both.
- A `dec` drives a field negative when the guard does not cover it. Pair `dec: k` with a `gte: k` guard to keep the field at or above zero.
- A range filter in the guard needs at least one defined bound. `{ stock: { gte: undefined } }` throws `StorageQueryError` rather than leaving the write unguarded.
- A losing writer that aborts instead of returning `{ applied: false }` throws `StorageSerializationError`, carrying the Postgres SQLSTATE (`40001` or `40P01`). Retry the call.

### Index Design
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ const result = await ctx.storage.submissions.query({
// Exact match
where: { status: "pending" }

// Range
// Range — needs at least one defined bound, or it throws StorageQueryError
where: { createdAt: { gte: "2024-01-01" } }
where: { score: { gt: 50, lte: 100 } }

Expand Down Expand Up @@ -155,6 +155,7 @@ Behavior to account for:
- `delta` accepts integers. A float throws `TypeError`.
- Pass at least one of `set` or `delta`. A field cannot appear in both.
- A `dec` drives a field negative when the guard does not cover it. Pair `dec: k` with a `gte: k` guard to keep the field at or above zero.
- A range filter in the guard needs at least one defined bound. `{ stock: { gte: undefined } }` throws `StorageQueryError` rather than leaving the write unguarded.
- A losing writer that aborts instead of returning `{ applied: false }` throws `StorageSerializationError`, carrying the Postgres SQLSTATE (`40001` or `40P01`). Retry the call.

### Index Design
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ const result = await ctx.storage.submissions.query({
// Exact match
where: { status: "pending" }

// Range
// Range — needs at least one defined bound, or it throws StorageQueryError
where: { createdAt: { gte: "2024-01-01" } }
where: { score: { gt: 50, lte: 100 } }

Expand Down Expand Up @@ -155,6 +155,7 @@ Behavior to account for:
- `delta` accepts integers. A float throws `TypeError`.
- Pass at least one of `set` or `delta`. A field cannot appear in both.
- A `dec` drives a field negative when the guard does not cover it. Pair `dec: k` with a `gte: k` guard to keep the field at or above zero.
- A range filter in the guard needs at least one defined bound. `{ stock: { gte: undefined } }` throws `StorageQueryError` rather than leaving the write unguarded.
- A losing writer that aborts instead of returning `{ applied: false }` throws `StorageSerializationError`, carrying the Postgres SQLSTATE (`40001` or `40P01`). Retry the call.

### Index Design
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ const result = await ctx.storage.submissions.query({
// Exact match
where: { status: "pending" }

// Range
// Range — needs at least one defined bound, or it throws StorageQueryError
where: { createdAt: { gte: "2024-01-01" } }
where: { score: { gt: 50, lte: 100 } }

Expand Down Expand Up @@ -155,6 +155,7 @@ Behavior to account for:
- `delta` accepts integers. A float throws `TypeError`.
- Pass at least one of `set` or `delta`. A field cannot appear in both.
- A `dec` drives a field negative when the guard does not cover it. Pair `dec: k` with a `gte: k` guard to keep the field at or above zero.
- A range filter in the guard needs at least one defined bound. `{ stock: { gte: undefined } }` throws `StorageQueryError` rather than leaving the write unguarded.
- A losing writer that aborts instead of returning `{ applied: false }` throws `StorageSerializationError`, carrying the Postgres SQLSTATE (`40001` or `40P01`). Retry the call.

### Index Design
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ const result = await ctx.storage.submissions.query({
// Exact match
where: { status: "pending" }

// Range
// Range — needs at least one defined bound, or it throws StorageQueryError
where: { createdAt: { gte: "2024-01-01" } }
where: { score: { gt: 50, lte: 100 } }

Expand Down Expand Up @@ -155,6 +155,7 @@ Behavior to account for:
- `delta` accepts integers. A float throws `TypeError`.
- Pass at least one of `set` or `delta`. A field cannot appear in both.
- A `dec` drives a field negative when the guard does not cover it. Pair `dec: k` with a `gte: k` guard to keep the field at or above zero.
- A range filter in the guard needs at least one defined bound. `{ stock: { gte: undefined } }` throws `StorageQueryError` rather than leaving the write unguarded.
- A losing writer that aborts instead of returning `{ applied: false }` throws `StorageSerializationError`, carrying the Postgres SQLSTATE (`40001` or `40P01`). Retry the call.

### Index Design
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ const result = await ctx.storage.submissions.query({
// Exact match
where: { status: "pending" }

// Range
// Range — needs at least one defined bound, or it throws StorageQueryError
where: { createdAt: { gte: "2024-01-01" } }
where: { score: { gt: 50, lte: 100 } }

Expand Down Expand Up @@ -155,6 +155,7 @@ Behavior to account for:
- `delta` accepts integers. A float throws `TypeError`.
- Pass at least one of `set` or `delta`. A field cannot appear in both.
- A `dec` drives a field negative when the guard does not cover it. Pair `dec: k` with a `gte: k` guard to keep the field at or above zero.
- A range filter in the guard needs at least one defined bound. `{ stock: { gte: undefined } }` throws `StorageQueryError` rather than leaving the write unguarded.
- A losing writer that aborts instead of returning `{ applied: false }` throws `StorageSerializationError`, carrying the Postgres SQLSTATE (`40001` or `40P01`). Retry the call.

### Index Design
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ const result = await ctx.storage.submissions.query({
// Exact match
where: { status: "pending" }

// Range
// Range — needs at least one defined bound, or it throws StorageQueryError
where: { createdAt: { gte: "2024-01-01" } }
where: { score: { gt: 50, lte: 100 } }

Expand Down Expand Up @@ -155,6 +155,7 @@ Behavior to account for:
- `delta` accepts integers. A float throws `TypeError`.
- Pass at least one of `set` or `delta`. A field cannot appear in both.
- A `dec` drives a field negative when the guard does not cover it. Pair `dec: k` with a `gte: k` guard to keep the field at or above zero.
- A range filter in the guard needs at least one defined bound. `{ stock: { gte: undefined } }` throws `StorageQueryError` rather than leaving the write unguarded.
- A losing writer that aborts instead of returning `{ applied: false }` throws `StorageSerializationError`, carrying the Postgres SQLSTATE (`40001` or `40P01`). Retry the call.

### Index Design
Expand Down
Loading