Repository navigation
fix(gateway): the approval list refuses an empty filter, and approval creation checks its body's types (N32 follow-up) - #513
Merged
Conversation
… creation checks its body's types (N32 follow-up) Follow-ups to PR #362 (N32) from its r2 SHIP verdict (rm-n32-362-r2-7fccd046.astra.verdict.md, "New findings, ranked"). M1 - GET /api/operator/approvals treated an explicitly empty or whitespace-only kernelId/status as absent, silently broadening the query past what the caller asked for (unsafe beside the OT-2 poller, which executes every returned record). Reproduced at 7fccd04: two kernels with approved records, `?kernelId=&status=approved` returned 200 with both records instead of 400; a whitespace-only kernelId and an empty `status` alone showed the same class of bug. Fixed by rejecting an empty/whitespace-only kernelId or status with 400 invalid_query, before any store read, matching the existing repeated-parameter check in the same handler. M2 - GET /api/operator/certifications said certifications are not recorded and that no certification store exists, but machine registration persistently carries the registrant's own certification claims (db/schema/onboarding.ts). Reproduced by inserting a registration with a nonempty operator.certifications and observing the old wording ("not recorded" / "no certification store") contradict it. Reworded the 501 to say there is no verified operator-certification read, and that certifications given at registration are the registrant's own claims, stored with the registration and not verified. Status and error code unchanged. Updated the one pre-existing assertion that pinned the old "not recorded" wording for this route (the wording itself was the subject of the fix). M3 - POST /api/operator/approvals only cast its body, never validated it. Reproduced: `capabilityType: {"unexpected": true}` was accepted (200) and stored verbatim; a non-string agentId, a parameters array, and an explicit `parameters: null` were each silently accepted or substituted instead of rejected. Fixed by requiring capabilityType (when present and not null) to be a non-empty string, kernelId and agentId to be non-empty strings (same "kernelId and agentId required" message), and parameters (when present) to be a plain object, never an array or null. autoApprove handling is unchanged (owned by gateway WP-C, #445). L1 - The operator.ts source ratchet is a narrow check for named fixtures, not a general fabrication guard. Reworded its comments (and the equivalent comment on the five-named-field check in the fixture describe block) to say so and point at the behavioral contract tests as the real protection. No test logic changed. New file packages/gateway/src/__tests__/operator-n32-followups.test.ts reproduces all three findings at 7fccd04 (9 failing / 1 passing control) and passes after the fix (10/10). Full gateway suite: 189 test files, 3013 passed + 6 skipped, no Errors line. tsc --noEmit: clean. Mutation check (undo one fix in a scratch copy, confirm the new tests fail, restore, cmp byte-identical) passed for M1 and M3. subagent: implementer-echo agent: pcc-readmodels (c255d7dc)
…(N32, #513 r1 M3) Cross-family review r1 of #513 (rm-n32-513-r1-cf16f558): M1, M2 and L1 CLOSED; M3 OPEN. An explicit `capabilityType: null` skipped the type check and still stored an approval (200). Reproduced at cf16f55 by a new case in operator-n32-followups.test.ts, before the fix: expected 200 to be 400. Only an absent field now means "unspecified". null, like any other non-string or an empty string, is a 400 invalid_body, and nothing is stored. Reverting this one condition is the cf16f55 state the reproduction failed on. agent: pcc-readmodels (c255d7dc)
LamaSu
changed the base branch from
fix/operator-routes-no-fabrication
to
master
October 3, 2026 22:13
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
These are follow-ups to #362 (N32), from its round-2 cross-family verdict (
rm-n32-362-r2-7fccd046, SHIP). This PR is stacked on #362's branch. Retarget it tomasterafter #362 merges.Each finding was reproduced at
7fccd046with a failing test before any fix; 9 of the new file's 10 tests failed there. The tenth is a positive control.GET /api/operator/approvals?kernelId=&status=approvedtreated the emptykernelIdas absent and returned every kernel's approved records. The OT-2 poller (scripts/ot2-agent.py:544) executes every record it gets. An empty or whitespace-onlykernelIdorstatusis now a 400invalid_query, decided before any store read.not_available) are unchanged.kernelIdandagentIdmust now be non-empty strings.capabilityType, when given, must be a non-empty string.parameters, when given, must be a plain object. Anything else is a 400invalid_body, and nothing is stored.autoApproveis unchanged; gateway's WP-C (WP-C: assurance ceiling + kernel owner checks (N31 e-stop, heartbeat, devices/N71, A2A owner) — stacked on #326 #445) owns it.operator-no-fabrication.test.tsnow say they are narrow named-fixture checks, and that the behavioral tests are the protection.Tests
packages/gateway/src/__tests__/operator-n32-followups.test.ts(10 tests).operator-no-fabrication.test.tsfollows M2's new text.tsc --noEmitwere run at this head.Agent: pcc-readmodels (c255d7dc).
🤖 Generated with Claude Code
https://claude.ai/code/session_01Sbd5dpwvmRsJqdff9zvNW6