Skip to content

fix(plugins): reject a storage range filter with no defined bound - #9

Merged
vedanshujain merged 1 commit into
feat/plugin-storage-updateiffrom
fix/storage-range-filter-guard
Sep 12, 2026
Merged

fix(plugins): reject a storage range filter with no defined bound#9
vedanshujain merged 1 commit into
feat/plugin-storage-updateiffrom
fix/storage-range-filter-guard

Conversation

@vedanshujain

Copy link
Copy Markdown
Owner

What does this PR do?

Targets feat/plugin-storage-updateif (emdash-cms#2169) rather than main, because it was found while reviewing that branch.

buildCondition pushes only the bounds that are defined, so a range filter whose every bound is undefined returned an empty SQL string. buildWhereClause then joined that empty string into the clause, dropping the predicate entirely.

The reachable shape is a bound built from an optional value:

const since = options.since;   // string | undefined
await ctx.storage.events.query({ where: { timestamp: { gte: since } } });

That type-checks, because every bound on RangeFilter is optional. Verified against the pre-fix code — three rows seeded, filter { stock: { gte: undefined } }:

query() returned 3 of 3 rows

So query() and count() returned the whole collection, and updateIf() applied its write with no guard at all:

updateIf("p1", { where: { stock: { gte: undefined } }, delta: { stock: { dec: 1 } } })
→ { applied: true }   row seeded at stock: 0 is now stock: -1

That last one is the oversell updateIf exists to prevent. A second field alongside the empty one emits a dangling AND and fails to parse.

buildCondition now throws StorageQueryError naming the field, and buildWhereClause skips any condition contributing no SQL. Callers that mean "match unconditionally" omit the field.

Note this changes behavior of the already-released query() and count(): a filter that previously matched everything now throws. That is the point of the fix — the old behavior was silent and failed open — but it is a behavior change on a shipped API, so it is called out in the changeset.

Type of change

  • Bug fix
  • Feature (requires maintainer-approved Discussion)
  • Refactor (no behavior change)
  • Translation
  • Documentation
  • Performance improvement
  • Tests
  • Chore (dependencies, CI, tooling)

Checklist

  • I have read CONTRIBUTING.md
  • pnpm typecheck passes
  • pnpm lint passes
  • pnpm test passes (or targeted tests for my change) — full packages/core suite, 6554 passed / 10 skipped / 0 failed
  • pnpm format has been run
  • I have added/updated tests for my changes (if applicable)
  • User-visible strings in the admin UI are wrapped for translation (if applicable) — n/a, no admin UI change
  • I have added and reviewed the user-facing changeset (if this PR changes a published package)
  • New features link to an approved Discussion — n/a, bug fix
  • I have included screenshots below if this PR changes the UI — n/a, no UI change

AI-generated code disclosure

  • This PR includes AI-generated code — model/tool: Claude Opus 5 (Claude Code)

Screenshots / test output

Not applicable — no UI change.

Three regression tests, each confirmed to fail before the fix:

  • storage-query.test.tsbuildCondition throws for an all-undefined range filter; buildWhereClause never emits a dangling AND.
  • storage.test.tsquery() rejects the filter instead of returning every row (the shipped path).
  • storage-updateif.test.tsupdateIf() throws instead of writing unguarded, and the row is unchanged.
Test Files  543 passed | 2 skipped (545)
     Tests  6554 passed | 10 skipped (6564)

buildCondition pushes only the bounds that are defined, so a filter whose
every bound is undefined returned an empty SQL string. buildWhereClause
joined that empty string into the clause, which dropped the predicate
entirely.

The reachable shape is a bound built from an optional value:

    const since = options.since;   // string | undefined
    await storage.query({ where: { timestamp: { gte: since } } });

That type-checks, because each bound on RangeFilter is optional. query()
and count() then returned every row in the collection, and updateIf()
applied its write with no guard — a guarded decrement could drive a
counter past the bound the caller asked for, which is the oversell
updateIf exists to prevent. A second field alongside the empty one
emitted a dangling AND and failed to parse.

buildCondition now throws StorageQueryError naming the field, and
buildWhereClause skips any condition contributing no SQL.

Callers that mean "match unconditionally" omit the field instead.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NK2NYBPGxwwzgHf7KfQRGM
@vedanshujain
vedanshujain merged commit bf9556f into feat/plugin-storage-updateif Sep 12, 2026
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant