Repository navigation
Public hygiene: steady spend-refusal test, strictjson range, RFC 3339 range note - #243
Conversation
TestSpendRefusalDoesNotEchoTheAmount looked for "777" as a substring of the serialized refusal and receipt input. The token id is the first 12 hex chars of a SHA-256 over a freshly minted token, so about 1 run in 400 the id held 777 (7775a9f2092b in CI run 38937620498) and the test failed with nothing echoed. amountEchoes decodes the document and checks every key, string and number on its own: a value echoes the amount if it contains the literal 777.31, if one of its plain-number words equals the amount, its whole part or its counter units, or if it is a JSON number equal to one of those. Identifiers that mix letters and digits are not compared. TestAmountEchoesIsExact pins the CI receipt (passes) and seven planted echoes plus a key echo (each flagged).
The strict reader bounded only the decimal exponent of a number's leading digit (at most 308), so 9e308, 1.8e308 and -9e308 were accepted although they are above math.MaxFloat64 and encoding/json refuses them as overflow (review comment 6095724731 on #227). In the top decade the magnitude is now compared exactly with math.MaxFloat64 and a larger one is refused with the same ErrCallUnreadable "number is outside the exact range" as 1e999. 1.7976931348623157e308 and everything below stay allowed. number() is the only place the reader parses numbers; it serves CheckCallLossless and CallHash (callers: mcpserver approval gate).
Reviewers ruled on #226 and #485 that parse_rfc3339 supports years 0001 to 9999. The docstring (identical in tools/ and demo/, which test_parse_rfc3339_identical_across_copies pins) now says so, including that a value whose UTC instant leaves that range after its offset is malformed. Both ends are already tested (ACCEPT 0001-01-01T00:00:00Z and 9999-12-31T23:59:59.999999999Z; RANGE_ERRORS year 0000 and the two offset overflows); they are now labelled. No behaviour change. demo/SOURCE-MANIFEST.json: sha256 of demo/verify_evidence_pack.py refreshed, as in 4744f27.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Independent review (profile satgate-security-reviewer, xai-oauth grok-4.7), round 1 Base: 348ed79. Tree: 1394aacc6fd38916858f0381d1b3c05d9adbc27a. Merge-base equals base. Did not approve, merge, label, deploy, or start Actions. Scope: 7 files (demo/SOURCE-MANIFEST.json, demo/verify_evidence_pack.py, pkg/mcpserver/spend_gate_test.go, pkg/toolcontrols/strictjson.go, pkg/toolcontrols/strictjson_test.go, tools/test_rfc3339_fraction.py, tools/verify_evidence_pack.py). git diff --name-only base..head -- satgate-landing/ is empty. Task A: amountEchoes (pkg/mcpserver/spend_gate_test.go:248) walks every key, string, and json.Number. It still flags decimal dollars (literal 777.31 and a plain-decimal word), whole dollars 777, and counter units 77731000000 (const units at spend_gate_test.go:167, same 1e8 as amountUnits). Nested maps and arrays are walked. Hex and mixed letter-digit words are not compared, so a token id that only contains the digits 777 is not an echo. Base go test ./pkg/mcpserver/ -run '^TestSpendRefusalDoesNotEchoTheAmount$' -count=3000 failed 10 times, all spend_gate_test.go:214, each token_id containing 777, zero line-219 hits. Head -count=3000 passed (1.842s). TestAmountEchoesIsExact passed. Two live plants in the OverLimit branch of pkg/mcpserver/spend_gate.go, each restored (worktree clean after): nested refusal data units 77731000000 failed at spend_gate_test.go:214 path Task B: number() (pkg/toolcontrols/strictjson.go:311) is the only number reader; value() at strictjson.go:122 sends every number there, and object/array recurse through value(). At lead10 == 308 (strictjson.go:393) the magnitude, sign stripped, is compared with math.MaxFloat64; larger returns the same unreadable() ErrCallUnreadable "number is outside the exact range" as 1e999 (strictjson.go:88). Checked on head and base, four wraps (object, array, nested, top): 9e308, -9e308, 1.8e308, 1e+309, 1E309, -0.9e309, 9E+308, 18e307, 99e307, 17976931348623158e292 all refused on head with that error; 1e309-class spellings were already refused on base. A 309-digit mantissa is refused on both as "number too long" (maxNumberText 64), still ErrCallUnreadable. In-range spellings accepted on both, including 1.7976931348623157e308, 1.7976931348623157e+308, 1.7976931348623157E+308, 1e+308, 1E308, -1.5e+308, 10e307, 17976931348623157e292, 0.17976931348623157e+309, 1.797693134862315708e308. 2.2e-308 is refused on both (pre-existing underflow bound, not this diff). 1.7976931348623158e308 is newly refused: its exact value is above MaxFloat64, though ParseFloat rounds it down to MaxFloat64. That is the documented stricter cutoff, not an in-range rejection. Callers CheckCallLossless (strictjson.go:62) and CallHash (approval.go:245; approval_gate.go:91 and :135) share readStrict, so they get ErrCallUnreadable. Task C: AST of both verify_evidence_pack.py files is unchanged once the new parse_rfc3339 docstring is stripped. tools/test_rfc3339_fraction.py adds one comment line above the existing 0001 and 9999 cases. Recomputed sha256 of demo/verify_evidence_pack.py is 74b6c5d56bed973da9bc6cd9c90d680d523ec26dcce3ba5f182eda853495ec02, equal to demo/SOURCE-MANIFEST.json:91. The old hash b35f5474 is gone from the tree. demo/tests/test_public_demo_static.py:153 recomputes the pin from the file bytes. satgate.io does not vendor the script: satgate-landing/app/.well-known/satgate/route.ts:65 points at the GitHub blob of tools/verify_evidence_pack.py, and the verify page only tells the reader to run that file. It does not need a site edit. The enterprise dashboard copy at dashboard/public/verify_evidence_pack.py (docs curl cloud.satgate.io/verify_evidence_pack.py) is an older fromisoformat parser, not this function. This PR does not change verifier behavior, so that served file does not need this docstring patch. Gates, from the head worktree: gofmt -l on the three changed Go files printed nothing (exit 0). go vet ./... exit 0. go test -race -count=1 ./pkg/toolcontrols/... ./pkg/mcpserver/... exit 0 (toolcontrols 2.461s, mcpserver 36.936s). /tmp/ph-venv Python 3.11.15: unittest discover -s tools -p 'test_.py' 70 OK; discover -s demo/tests -p 'test_.py' 8 OK. Same commands as .github/workflows/ci.yml:30 and :33. CI: gh pr checks 243 and the commit check-runs are green on d2b8076. Actions run 38951466423 (pull_request, conclusion success, headSha matches). Go job 117143620318 (log: ok pkg/mcpserver 52.117s, ok pkg/toolcontrols 8.953s). Node.js SDK job 117143620186. Python SDK job 117143620529. Vercel Preview Comments check-run 117143671526. Vercel deployment pass has no Actions run id. This public workflow has no wave1-guards job. PR is OPEN, not draft, MERGEABLE, mergeStateStatus BLOCKED, reviewDecision REVIEW_REQUIRED (branch policy, not a failed check). |
This branch was successfully deployed
1 participant
Head SHA: d2b8076 (base main 348ed79)
Three commits, one per task.
A. TestSpendRefusalDoesNotEchoTheAmount flaked (c76cfac)
Cause: the check was
strings.Contains(json(receipt input), "777").token_idis the first 12 hex chars of a SHA-256 over a freshly minted token, so it is random on every run. CI run 38937620498 attempt 1 failed at spend_gate_test.go:214 with"token_id":"7775a9f2092b"; nothing in that receipt carries the amount. On main,-count=4000failed 4 times (token ids 057777fcfd12, 3ac2e777c899, bb9f64a83777, ced40e777ff3).Fix:
amountEchoesdecodes the refusal and the receipt input (UseNumber) and checks every key, string and number on its own. A value echoes the amount if it:777.31(a decimal point cannot occur in a hex id),Words that mix letters and digits (hex ids, rcpt_1) are identifiers and are not compared. New
TestAmountEchoesIsExactuses the CI receipt as it was logged (it passes) and plants 7 echoes and a key echo. Each one is flagged at its own path.Proof:
-count=10000: 0 failures.-race -count=2000: 0 failures.token_idheld 777 (777bd4a15e8e, 00777057483e, 9d360cb77733). On each one the old check would fail and the new check found nothing.approval_id= amount →at [$.approval_id]control_symbol= units →at [$.control_symbol]at [$.error.message]asked: 777→at [$.error.data.asked]B. strictjson accepted numbers above math.MaxFloat64 (398e5ba)
Cause:
pkg/toolcontrols/strictjson.gonumber()bounded only the decimal exponent of the leading digit (at most 308). So 9e308, 1.8e308 and -9e308 passed, which review comment 6095724731 on #227 pointed out.number()is the reader's only number parser. It servesCheckCallLosslessandCallHash, which are called from the mcpserver approval gate (approval_gate.go:91 and :135).Fix: when the leading digit is at 10^308, the magnitude is compared exactly (big.Rat) with math.MaxFloat64. A larger number is refused with the same
ErrCallUnreadable"number is outside the exact range" as 1e999. The doc comments now give the right maximum (they said 1e308 was the largest finite binary64). The bound is exact: 1.7976931348623158e308 is refused, even though ParseFloat rounds it down to MaxFloat64, and 1.7976931348623157e308 is accepted.Proof:
TestNumbersAboveMaxFloat64AreRefusedchecks the error type and exact message from both entry points._meta, andargumentsitself.Run against main's strictjson.go, the test fails for every refused case except 1e309, which main already refused.
Note: at the MCP approval gate, main already refused 9e308 before
CheckCallLosslessruns.argrules.CheckCallShapewalks the call with encoding/jsonToken(), which overflows on that number. So this fix closes the gap for direct callers ofCheckCallLosslessandCallHash. I did not add a gate-level test because it passes on main.C. parse_rfc3339 range note (d2b8076)
The docstring now says years 0001 to 9999, as ruled on #226 and #485. It also says year 0000 is malformed, and so is any value whose UTC instant leaves that range once its offset is applied. Both copies (tools/ and demo/) carry the same docstring, which
test_parse_rfc3339_identical_across_copiesrequires. Tests at both ends of the range already existed, so I labelled them and added none. No behaviour change.The sha256 of demo/verify_evidence_pack.py in demo/SOURCE-MANIFEST.json is refreshed, the same way 4744f27 did it.
Local gates
gofmt -lon changed Go files: exit 0, no outputgo vet ./...: exit 0go test -race -count=1 ./pkg/toolcontrols/... ./pkg/mcpserver/...: exit 0python -B -m unittest discover -s tools -p 'test_*.py': exit 0 (70 tests)python -B -m unittest discover -s demo/tests -p 'test_*.py': exit 0 (8 tests)Python runs used 3.11.15 with rfc8785 0.1.4 and cryptography 50.0.0, the versions ci.yml installs.