Skip to content

[finding] DeleteDataRequestSchema is declared in packages/spec but never validated against — the DELETE data route bypasses request-schema validation entirely #13852

Description

@zhuangjianguo

Filed unassigned — recording only, no severity asserted, routing is triage's. Surfaced while implementing #13576 (A2.2/A2.3 measurement), unrelated to that fix and not touched by its PR.

What's declared

packages/spec/src/api/protocol.zod.ts declares DeleteDataRequestSchema (object, id, expectedVersion), documented and exported, sitting right beside UpdateDataRequestSchema.

What's enforced

UpdateDataRequestSchema is safeParse'd against the assembled PATCH request in packages/rest/src/rest-server.ts before the request reaches the engine — a non-conforming body 400s with a structured fields[] envelope (search UpdateDataRequestSchema in that file, the /data/:object/:id PATCH handler).

DeleteDataRequestSchema has zero safeParse/validation call sites anywhere in the tree. Confirmed with:

grep -rln "DeleteDataRequestSchema" packages/ content/

The only hits are packages/spec/src/api/protocol.zod.ts (the declaration itself), packages/spec/src/type-alias-convention.pin.test.ts, the two generated export-surface trackers (packages/spec/export-origins/api.json, packages/spec/api-surface/api.json), and the generated docs page (content/docs/references/api/protocol.mdx). No route, no handler, nothing under packages/rest/src or packages/metadata-protocol/src ever calls DeleteDataRequestSchema.safeParse(...) or equivalent.

The DELETE route (packages/rest/src/rest-server.ts, DELETE /data/:object/:id) builds its own ad hoc request shape (object, id, expectedVersion from If-Match/query) and hands it straight to protocol.deleteData(...) with no schema gate in front — unlike its PATCH sibling.

Why this looks like a real gap, not a design choice

This is the same shape ADR-0049 (enforce-or-remove) and Prime Directive #10 ("declared ≠ enforced") care about elsewhere in this repo: a schema that exists, is exported, is documented, and validates nothing. Practically the blast radius looks narrow today — object/id are path params Express already type-checks as strings, and expectedVersion's ad hoc handling (refuseRepeatedQueryParams, the If-Match/query-precedence logic) covers some of what the schema would — but the schema itself is not what's doing any of that work, and a future field added to DeleteDataRequestSchema would silently be unenforced on this door while looking enforced everywhere the type is referenced.

What I did NOT do

Did not fix this — out of scope for #13576, unrelated defect class, and #13576's own fix (the OCC malformed-token check) lives inside packages/metadata-protocol's assertVersionOf/assertVersionMatch, which both PATCH and DELETE reach regardless of whether DeleteDataRequestSchema is ever parsed — so this gap does not affect #13576's correctness.

Not claimed

Did not check whether other DELETE-shaped routes in this repo have the same gap (this is scoped to the single /data/:object/:id DELETE data route found while tracing #13576). Did not assess severity or decide the fix (add the safeParse call to mirror PATCH, or retire the unused schema) — that's triage's call.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    domain:clipm:retriageQuestion for triage, answered each fire; coexists with the standing pm:* label; no dispatchpriority:p3

    Type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions