Skip to content

fix(plugin-approvals)!: sys_approval_token, the action-link tokens, is no longer exposed through the automatic API (#22616) - #22633

Merged
objectstack-fleet[bot] merged 6 commits into
mainfrom
claude/issue-22616-approval-token-generic-door
Oct 10, 2026
Merged

objectstack-fleet[bot] merged 6 commits into
mainfrom
claude/issue-22616-approval-token-generic-door

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #22616
Clause-②: no (narrowing)

What this does

sys_approval_token, the single-use action-link tokens of ADR-0043, is no longer exposed through the automatic API. Its enable block moves from apiMethods: ['get', 'list'] to apiEnabled: false with apiMethods: []. The generic doors now serve token rows to no caller, as the approvals door (/api/v1/approvals/*) serves them to nobody.

The object's own declaration already said its tokens are "minted and consumed by the approval engine (SYSTEM_CTX), never via the data API". What is declared is now what is enforced. The engine's mint, lookup-by-digest and consume path runs as the system and is unchanged.

This is the third request-keyed table of the family, after #22559 (sys_approval_request, PR #22587) and #22589 (the child tables, PR #22613).

Step 1a: reach, measured before any code

Measured through the real HTTP doors on an in-process stack: @objectstack/verify's bootStack with the real SecurityPlugin, automation, the record-change trigger and ApprovalsServicePlugin.

The scene:

  • one approval flow whose approver is a concrete identity;
  • one pending request, and one reminder from its submitter, which minted one approve row and one reject row (ADR-0043);
  • a member who takes part in no request and holds an app permission set granting read on the token object;
  • a second member with no such grant, as the control.

The probe was a local scratch file and is not part of this PR.

caller (class) door before (83b8b80728) after (9ecf41b2c7)
member with a read grant, in no request REST list 200, both rows (total 2): request, bound identity, action, expiry, consumption state 404 OBJECT_API_DISABLED
same REST by id 200, the row 404 OBJECT_API_DISABLED
same REST query 200, both rows (total 2) 404 OBJECT_API_DISABLED
same REST export 403 (no export grant) 404 OBJECT_API_DISABLED
same cross-object search, scoped to the object, and unscoped 2 token hits each 0 token hits
administrator REST list, by id, query 200, every row 404 OBJECT_API_DISABLED
administrator cross-object search 2 token hits 0 token hits
member, no grant (control) REST list, by id, query 403 PERMISSION_DENIED 404 OBJECT_API_DISABLED
member, no grant (control) cross-object search 0 hits 0 hits
submitter and approver, no grant REST list, by id, query 403 PERMISSION_DENIED 404 OBJECT_API_DISABLED

Step 1b: readers census

surface reading reader through a generic door?
this repo: packages/, apps/, examples/, the dogfood suites (git grep sys_approval_token) the approval service (mint, lookup by digest, consume, all as the system); the lifecycle hooks' own-object exclusion list; the plugin's manifest and nav test; the spec's platform-object name list; the tenancy census; the internal-hash test (an engine read as the system) none
the pinned console: objectstack-ai/objectui at .objectui-sha 20c6d351ad (full tree) sys_approval_token, approval_token, approvals/act: 0 hits. Control: sys_approval_request has hits none
Setup and Studio the plugin's nav contribution lists the inbox, requests, actions and delegations; no entry, view or page names the token object none
what an administrator is served today every token row, by list, id, query and search (table above) served, but nothing consumes it
docs/qa/platform-checklist/areas/approvals.json, item approvals.email-action-token-door an internal QA procedure that reads token rows as the administrator through the data API, as an oracle a procedure, not a product reader; see Acceptance notes

No product reader exists, so no visibility rule for tokens is invented to keep one.

Why this spelling (read from the source)

  • apiMethods has three declared states. See packages/spec/src/data/api-derivation.ts, its module doc and resolveEffectiveApiMethods:

    • undefined resolves to unrestricted (every operation);
    • [] resolves to deny-all (fully closed);
    • a subset resolves to its derived closure.

    An absent whitelist is fully open, so deleting the key would retire nothing.

  • apiEnabled is the declared off switch: ObjectCapabilities.apiEnabled, "Expose object via automatic APIs", default true. apiExposureDenialReason judges it first, for every operation. REST (apiAccessDenialFromEnable, answering 404 OBJECT_API_DISABLED), the dispatcher (checkApiExposure) and the MCP bridge (enforceApiExposure) do the same.

  • The cross-object search reads only the switch. ObjectStackProtocolImplementation.searchAll skips an object on searchable === false or apiEnabled === false, and never reads apiMethods. So apiMethods: [] alone would leave the search door serving the rows. Ablation C below measures exactly that.

  • Precedent. Every credential and token store already declares both: sys_flow_credential, the sys_oauth_* stores and sys_jwks. setup-nav.contributions.ts calls apiMethods: [] fail-closed by design.

Both halves are spelled. apiEnabled: false closes the doors, and apiMethods: [] makes the whitelist agree with it instead of advertising get and list behind the switch.

Pins

The pins are in packages/plugins/plugin-approvals/src/sys-approval-token-generic-door.integration.test.ts.

  • The declaration: apiEnabled: false, apiMethods: [], effective mode deny-all.

  • Every operation is refused, for every caller. apiExposureDenialReason refuses every operation in API_OPERATION_ORDER (14, floored) with the api-disabled discriminant, and bulk with each child verb. This function is the decision every door turns into its refusal. canServeApiOperation serves none, and OBJECT_API_DISABLED is registered vocabulary. The function takes no caller, so this is the granted member's answer and the administrator's alike.

  • The cross-object search, on a real engine. The rig: ObjectQL over better-sqlite3, the real ApprovalsServicePlugin.start() and the real protocol, with two token rows minted by remind(). No token row comes back for a member holding read or for an administrator:

    • searching by the bound slot or by the request;
    • scoped to the object, or swept beside other objects.

    Control: the search still finds the business record.

  • Positive control. The Approve link that remind() delivered peeks, redeems once as the bound approver, and is refused as consumed on replay. The engine's system read still sees both rows, with the redeemed one consumed, and the request is approved.

This package does not depend on @objectstack/rest or @objectstack/verify, the same choice as the webhook object's exposure test. So the HTTP readings above come from the scratch probe. The 404 OBJECT_API_DISABLED envelope for this discriminant is pinned in @objectstack/rest's and @objectstack/mcp's own suites.

Red first, then ablation

The subject is imported from source, so no build sits between a mutation and a run.

  • Each mutation went through scripts/ablation-replace.mjs: the anchor must hit exactly once, and the blob must change.
  • Each restore was proven by the blob hash equalling HEAD's (abe8c71c89), with git diff HEAD empty.
run declaration declaration pin every-operation pin search pin scene, search control, positive control
red first (c8584b9c23, before the fix) original red red red green
fix (9ecf41b2c7) apiEnabled: false, apiMethods: [] green green green green
ablation A: both halves back to the original apiMethods: ['get', 'list'] red red red green
ablation B: get and list restored, switch kept apiEnabled: false, apiMethods: ['get', 'list'] red green green green
ablation C: switch dropped, whitelist kept apiMethods: [] red red (method-not-allowed, not api-disabled) red: the search serves both token rows green
  • B: the switch is what closes the doors; restoring get and list alone reopens nothing.
  • C: apiMethods: [] alone leaves the search open.

Tests and gates

All readings are at dd037479aa unless stated.

  • Package tests. pnpm --filter @objectstack/plugin-approvals exec vitest run --maxWorkers=2: 68 files and 994 tests passed, at 27c94553c5. The one later commit only types a query-options bag in the new test file. That file was re-run at dd037479aa together with the internal-hash test: 7 of 7.
  • Package typecheck. pnpm --filter @objectstack/plugin-approvals run typecheck (tsc --noEmit, the scripts project and check:test-typecheck): exit 0. The test layer compiles, with its identity-pinned debt unchanged (8 files, 324 errors, 27 signatures).
  • Spec repo tests. pnpm --filter @objectstack/spec run test:repo, which scans every object's apiMethods: 54 files and 915 tests passed, at 27c94553c5. Nothing it reads changed afterwards.
  • Derived gates. node scripts/pm/dispatch-gates.mjs --commands derives 65 commands for this diff. All 65 exited 0, each exit captured before any pipe. The --ran reconciliation reads 65 derived, 65 run, 0 unrun. Among them:
    • check-adr-0087-registration: one declared-breaking changeset, carrying its disposition;
    • check-changeset-no-major: no major;
    • check:i18n: 9 packages in sync;
    • check:query-options-erasure: the test surface is back at its 236 ceiling after typing one bag;
    • check:test-source-alias, check:cross-package-test-inputs and check:nul-bytes.
  • Lint, narrowed with proof. eslint --no-inline-config over the two changed .ts files.
    • Both are inside the linted population: --print-config resolves 6 and 5 rules.
    • --format json reports 2 files, 0 errors and 0 warnings.
    • Type-aware linting is not enabled (no parserOptions.project or projectService), so this diff cannot move any untouched file's verdict.
    • The repo-wide pnpm lint is CI's.

Acceptance notes

  • The analytics door is not closed by this change. This is out of scope here and has been reported to the seat as a security finding.
    • The analytics ad-hoc query infers a cube for any object name (inferCubeFromQuery). It reads neither the object-level apiEnabled: false nor the field-level internal: true.
    • Measured on the same stack after this change: a member holding read on the token object, and an administrator, are still served its rows as dimension rows, and its withheld digest column.
    • The same holds for another apiEnabled: false credential store's internal column, served to an administrator.
    • It is a different door, owned by service-analytics, and it reaches every object with such a declaration.
  • A QA procedure now contradicts the door. The checklist item approvals.email-action-token-door reads token rows as the administrator through the data API, as an oracle, and writes expires_at through it for its expiry leg.
  • The cross-object search never reads apiMethods. An object whose whitelist omits list is still swept unless it also declares searchable: false or apiEnabled: false. This is dormant today: the three whitelists without list (sys_verification, sys_device_code, sys_two_factor) all declare searchable: false. Ablation C shows the shape on a counterfactual. Noted, not filed.
  • No HTTP-level pin is committed. The claim scoped tests to plugin-approvals, which cannot depend on @objectstack/rest or @objectstack/verify. An end-to-end pin would live in packages/verify or the dogfood suite.
  • /me/permissions annotates this object with an empty operation set wherever the subject's map carries it (annotateEffectiveApiOperations reads canServeApiOperation). This was read from the source, not measured.

Generated by Claude Code

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Oct 10, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

Nothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 1 changed package(s)), so this run has no opinion about the docs.

What this run could not see

Coarse fallback — 6 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json f368b7e980d3df201620098be965523b50d6df19 → packageMentionDocs.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: dd037479aa1af849a2af54907082b17062ddb24c
Local-runs: none

Inputs: card #22616 (body and all three comments, the os-dev-report 6095111901 included), PR #22633 (body, file list, net diff against origin/main at its merge base 83b8b80728), and the check-runs on the head. Carried items read: #22634 and #22631 with its comment 6095140961. Nothing built, run or re-run; every source position below was read from the tree at the head sha.

Check-runs on the head, read at 2026-10-10T07:44Z: 34 distinct names, all completed, none red. The seven required contexts are success: Lint & Repo Gates, TypeScript Type Check, Test Core, Dogfood Regression Gate, Build Core, Temporal Conformance (live PG + MySQL), Governed Surface Queue Guard. Build Docs, Console Pin Gate and Packed-tarball smoke (opt-in) are skipped (path-filtered or opt-in; no spec export moves, so the pin gate owes nothing). No governed path is in the file list.

① Derived judgments

The diff is three files: the object declaration, one integration test, one changeset. Every accept-set and public-surface change it implies, named and judged:

  1. enable.apiEnabled: false on sys_approval_token — the automatic API's off switch. Accept set: every operation on the object through the REST data routes, the runtime dispatcher and the MCP data bridge is refused for every caller, the administrator included. Right. Verified from source: apiExposureDenialReason (packages/spec/src/data/api-derivation.ts, the apiEnabled === false test ahead of the whitelist) is the one decision; REST turns it into 404 OBJECT_API_DISABLED in apiAccessDenialFromEnable and enforceApiAccess (packages/rest/src/rest-server.ts); the dispatcher's checkApiExposure (packages/runtime/src/api-exposure.ts) and the MCP enforceApiExposure (packages/mcp/src/stdio-data-bridge.ts) judge the same switch first. The readers census holds at the head: a git grep of the object name finds only the approval service (system-context mint, lookup by digest, consume), the lifecycle hooks' exclusion list, the nav and bundle tests, the spec's platform-object name list, the tenancy census, the internal-hash test (which reads through engine.find as the system, not a door) and one internal QA procedure. The precedent credential and token stores (sys_flow_credential, the sys_oauth_* stores, sys_jwks) declare the same switch. The object's own ADR-0103 comment said never via the data API; what is declared is now what is enforced.

  2. enable.apiMethods: ['get', 'list'] to [] — the whitelist narrows from read-only to deny-all. Right. The three-state table in api-derivation.ts makes an absent key unrestricted, so deleting it would have retired nothing; [] is the declared closed state. ADR-0103 D3's reconcileManagedApiMethods (packages/objectql/src/registry.ts) strips write verbs only and returns an empty whitelist untouched, so nothing re-opens it at registration; D5's ['get', 'list'] lock is specific to sys_import_job, so no recorded decision is reversed (Prime Directive 13). The declaration pin holds both halves and the resolved mode.

  3. The cross-object search no longer sweeps the object, for any caller. Right. searchAll (packages/metadata-protocol/src/protocol.ts) skips an object on searchable === false or apiEnabled === false and never reads apiMethods; the PR's ablation C measured exactly that counterfactual, and the real-engine search pin covers the bound slot and the request id, scoped and swept, for a member and an administrator, with a positive search control. Note, not a gap: the precedent stores also declare searchable: false and this PR does not. The only other runtime reader of that flag is the ObjectQL search companion (packages/objectql/src/search-companion.ts), a storage column whose sole reader is the find path, which the doors now refuse; at worst a dead companion column is provisioned on the token table.

  4. The no-grant control moves from 403 PERMISSION_DENIED to 404 OBJECT_API_DISABLED. Right, and not authored by this PR. The REST list route runs enforceAuth, then enforceApiAccess, then the data call where the permission check lives (rest-server.ts, the data-route registration), so the exposure gate is judged before the grant by the pre-existing two-step order. A uniform 404 for an object that is not exposed is the stronger posture: a 403 would confirm the object's existence and let a caller distinguish granted from not granted. The dispatch named the no-grant case as a measurement control, not as a required answer; the dev reported the deviation in the report, the PR body and the changeset.

  5. The apiOperations annotation on the effective-permissions payload. annotateEffectiveApiOperations (packages/core/src/security/effective-object-permissions.ts) reads canServeApiOperation, so the object's entry carries an empty set wherever a subject's map carries it. Right: the machine-readable surface advertises nothing the door refuses. Read from source, as the dev stated; not measured.

  6. The engine's own token path is unchanged. Mint, lookup by digest and consume run as the system through the engine, not through any door; the positive-control pin redeems a minted link once, is refused as consumed on replay, and reads both rows back as the system with one consumed and the request approved. Right.

  7. No gate was added. APPROVAL_REQUEST_CHILD_OBJECTS in request-read-gate.ts still names the action and approver tables only; approvals-plugin.ts is untouched. The dispatch forbade a gate unless the census forced one, and it did not. Right.

  8. Surfaces that do not move: the OpenAPI document is the static spec shipped from @objectstack/spec, not per-object; /discovery is route-level; the i18n bundles carry no label from the enable block (check:i18n ran green inside Lint & Repo Gates). Residual nit: the #21197 comment above token_hash still speaks of "this object's get/list doors", which no longer exist; harmless.

Pins against the card's Ask 3: a granted member is served no token row by list, by id or by count. The pin sits at the decision every door delegates to, over every operation in API_OPERATION_ORDER (count rides list and aggregate, both in it) and the bulk children, plus the real-engine search pin; the red-first run and ablations A, B and C are recorded with the restore proven by blob hash. The 404 OBJECT_API_DISABLED envelope is pinned in the REST and MCP suites at the head (rest-api-derivation-gates.test.ts, rest-method-not-allowed-conjunct.test.ts, stdio-data-bridge.exposure.test.ts).

② Semver level

  • Publishes: yes. @objectstack/plugin-approvals is a released package and the object declaration ships in it, so a changeset is owed and skip-changeset would be wrong. The changeset is present: '@objectstack/plugin-approvals': minor.
  • Clause-②: no (narrowing) in the PR body and in the changeset body, and they agree. A narrowing is BREAKING, and the changeset says so.
  • BREAKING as minor: right. scripts/check-changeset-no-major.mjs is the launch-window guard and refuses a major bump; minor is the highest level available and the standing convention, named in the changeset body. patch would understate a declared BREAKING change. Check Changeset and the no-major gate inside Lint & Repo Gates ran green.
  • FROM to TO: carried. The declaration, each door's answer, the no-grant answer and the search are each stated, and "what changes for you" says there is no replacement read by design, with the act door's outcome pages and the request's action history named as where a link's state lives.
  • ADR-0087 marker: not-required (no-migration-prescription), judged right on substance. No metadata an author writes moves: no spec key, export or config field is removed or renamed, no stored row is read, rewritten or dropped, so objectstack migrate meta has nothing to rewrite, and the body ships no consumer rewrite (the FROM to TO describes the door's behaviour, not a code edit). The marker closes the other four categories on facts, and each holds: the bumped package publishes (not unpublished); no ADR-0087 id covers this path and the diff adds none (not registered, not already-registered); nothing exported changes (not runtime-interface-only, not type-surface-only). check-adr-0087-registration ran green.
  • Clause-②: no (narrowing) stands as declared; this record is the one contract-tier review it owes.

③ Boundary flags

Open question 1 — amend the internal QA checklist item here. Answered: B. The item approvals.email-action-token-door (docs/qa/platform-checklist/areas/approvals.json) reads token rows through the data door as the administrator in two steps and backdates expires_at through it in its expiry leg; after this PR those steps answer 404, and two of them already contradicted the door (the digest is withheld since #21197; update was never in the whitelist). The carrier is verified: #22631 covers the same file, and its comment 6095140961 names this item, the 404, and the remedy (move the oracles to the act door's outcome pages, the request's action history and the internal-hash test), to ride that card's non-governed half after this PR merges. Extending a security PR's file surface to a QA text with a filed carrier is not warranted; the dev's concern (a later run reading the 404 as a defect) is met by the carrier being on file before the landing. Not escalated.

Open question 2 — an HTTP-level pin. Answered: A, no further pin is a condition of this PR. The decision every door turns into its refusal is pinned over every operation; the real-engine search pin covers the one door that reads the switch and not the whitelist; the REST and MCP envelopes are pinned in their own suites; and HTTP reach was measured before and after on a real stack with the readings in the PR body. plugin-approvals depends on neither @objectstack/rest nor @objectstack/verify, the same choice the webhook object's exposure test made. A bootStack pin in packages/verify would add a surface without adding a fact. Not escalated.

Out-of-scope finding 1 — the analytics door. Verified as #22634 (security, priority:p1, domain:services, pm:dispatched): the ad-hoc analytics query mints a cube for any object name (inferCubeFromQuery, packages/services/service-analytics/src/analytics-service.ts) and the package source carries no reference to apiEnabled or to a field's internal flag, so it serves rows of an apiEnabled: false object and columns declared internal: true, the withheld digest among them. Is this PR's narrowing honest while that door stays open? Yes, as framed. The changeset's title says "no longer exposed through the automatic API", its FROM to TO enumerates exactly the doors it closes (the REST data routes, the dispatcher, the MCP data tools, the cross-object search), and every one of them is closed and pinned; the PR body's acceptance notes disclose the analytics door in the open, and the seat filed it at p1 before this PR lands, ordered independent of it. One residual, named and not blocking: the sentence "the generic doors serve token rows to no caller" uses a plural that reads one step wider than the enumeration beneath it while #22634 is open. The honest closing of that sentence belongs to #22634's own changeset when it lands; holding this PR would keep the data door open for no gain.

Out-of-scope finding 2 — the QA checklist. Carried by #22631, as above. Answered.

Out-of-scope finding 3 — searchAll never reads apiMethods. Verified from source at the head: the skip predicate reads searchable and apiEnabled only, so a whitelist that omits list does not keep an object out of the cross-object search by itself. The dormancy claim holds: sys_verification, sys_device_code and sys_two_factor each declare searchable: false beside apiMethods: ['get']. Escalated to the dispatching seat: file it. The dev's disposition is carrier: none, noted, not filed. Prime Directive 10 files a contract violation; dormancy is a fact about today's objects, not a disposition. The spec derives search from list, so an object whitelisting ['get'] declares that search is not served, while the protocol's sweep would serve it the moment one such object lacks searchable: false. That is declared-not-enforced, and the remedy is the one #22634 already states for its door: the sweep consults canServeApiOperation for search rather than keeping a second rule. Outside this PR's file surface and not a condition of its PASS.

Dispatch constraints: no packages/spec, no request-read-gate.ts, no approvals-plugin.ts, no content/docs/releases/: all held. The branch and PR assignee match the claim; the PR is a draft on an internal branch (head repo equals base repo).

Implemented-by: claude/issue-22616-approval-token-generic-door
Reviewed-by: session_013j5gkUCpqQiti4GgPqqmnt (contract-review subagent, CONTRACT_REVIEW_TIER)

VERDICT: PASS

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

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants