CRW-347: port the hook trust identity hash - #560
Merged
Merged
Conversation
HookTrustIdentityHash is a stub here that returns an empty string, so 78 assertions fail against the CXC v0.2.40 answers that testdata/hooktrust/record-identity.mjs recorded into testdata/hooktrust/identity-oracle.json. The port follows in the next commit.
HookTrustIdentityHash follows identityHash (hook-trust.ts:18-38, 40-47, 96-139) as-is: the ten event labels, the six refusal messages, the handler normalized to type command with the JSON-spelled clamped timeout into pyjson.Dumps, and the two kept oracle behaviours (the Object.prototype event names and the raw toString TypeError) recorded in docs/port-cxc/known-defects.md. go test -count=1 ./internal/runtime/doctor passes against the answers recorded from the v0.2.40 dist.
|
You have reached your Codex usage limits for security reviews. Please try again later. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
…rogates A json.Number past the double range parses to the infinity JavaScript has, so the type refusal now spells Infinity and -Infinity as the oracle does, and the timeout refusal is unchanged; three recorded cases pin it. The doc comment no longer implies encoding/json preserves every valid hook document, and a test shows the pyjson reading of a lone surrogate matching the recorded oracle hash while the standard reading differs.
staticcheck must not rewrite the verbatim message the oracle throws for an object type with an own toString member; the repository idiom is the lint directive.
The coordinator refreshed the base. The only conflict was in docs/port-cxc/known-defects.md, where both sides appended lines at the same place (1 block(s)); both sets are kept, dev's lines first, then this branch's.
This was referenced Oct 5, 2026
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
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.
What this changes
internal/runtime/doctor/hooktrust_identity.goaddsHookTrustIdentityHash(event string, matcher *string, handler map[string]any) (string, error), the read-only port of CXC v0.2.40hook-trust.ts:18-38(EVENT_LABELS, MATCHER_DROPPED_EVENTS),:40-47(HookHandler) and:96-139(assertSupportedPlatform, sorted, identityHash) at commit3c1459ac. It returnssha256:plus the hex digest of sha256 over the canonical JSON of a command hook's trust identity; it reads no file, holds no package state, needs no Node and no CGO, and does no package-level initialization.Behaviour, as the oracle has it:
unsupported hook event: <name>,unsupported hook handler type: <value>,hook command must be a string,hook timeout must be a finite number,hook async must be a boolean,hook statusMessage must be a string), in the oracle's order;{type: "command", command, timeout: max(timeout or 600, 1), async: async or false}, withstatusMessageonly when it is not null;UserPromptSubmitandStoponly, an empty matcher included;internal/pyjson.DumpswithCompact,SortKeysandUnicode; the clamped timeout is pre-spelled the way JavaScript'sJSON.stringifyprints a number (600,1.5,1e+21,10000000000000000) and handed toDumpsas ajson.Number, becausepyjson.Floatis Pythonrepr(600.0).json.Numberpast the double range reads as the infinity JavaScript holds, so a refusedtypeof1e400refuses asunsupported hook handler type: Infinity; the timeout refusal is unchanged.Two oracle behaviours are kept as-is and recorded in
docs/port-cxc/known-defects.md(one line each,port: kept): an event named after a JavaScriptObject.prototypemember is accepted (the eleven function members hash withoutevent_name,__proto__withevent_name: {}), and an objecttypewith an owntoStringmember makes the type refusal throw the raw TypeErrorCannot convert object to primitive value.Criteria
go test, and the six ported B tests stand as Go tests;481075124: 78 assertion failures with the stub), then green at the head;contract/fixtures/cxcfixture is driven by this unit alone, socontract/notes/cxc/CRW-347.jsonclaims none;Verification
Red first: commit
481075124holds the tests plus a stub, andgo test -count=1 ./internal/runtime/doctorfails with 78 assertion failures (exit 1) against the recorded oracle answers.Green at head
bcea10a336a355f827830d9e073663487896cec9:go test -count=1 ./internal/runtime/doctor- ok (the six ported B tests, the 68 recorded cases, and the Go-side branch cases JSON cannot carry: NaN, the infinities, negative zero, and the refusal order)go test -count=1 ./internal/contracttest -run '^TestDomain/cxc$'- ok 6.332sCGO_ENABLED=0 go build ./...andgo vet ./internal/runtime/doctor- exit 0;go vet ./...andGOOS=darwin go vet ./...- exit 0 on the tree before the review fix, whose commit touches only this package (its vet reran clean) and whose whole tree is covered by the hosted CI run belowcrw-dev ci validate,crw-dev ci contracts- exit 0crw-dev ci plugin-Package 0.4.0+adfc99ffc96f ...: 216 files, digest 635105c68ea07f89, the same version and digest the base5c929a6e14prints, and no working-tree drift line: the activation surface is untouchedgofmt -l internal/runtime/doctorempty;git diff --check origin/dev...HEADemptyorigin/dev..HEAD(3 commits): no leaksCRW CIrun37253119101, dispatched on this exact head because the pull request conflicts only in the append-onlydocs/port-cxc/known-defects.md(GitHub starts nopull_requestrun for that); every job must be green on it before the handoffReview round
Devin reviewed
d49d09a4once and found two yellow items, both fixed inbcea10a33and answered and resolved on their threads:typespelling (1e400) fell through to%vinstead of the oracle'sInfinity: the range-error value is now kept,-1e400refuses as-Infinity, and three recorded cases pin it;encoding/jsonreads it, which is a reader boundary, not something the unit can recover: the doc comment now namespyjson.LoadswithLoadOptions{Surrogates: true}as the surrogate-preserving reader, one recorded case is asserted through that reader, and the standard reader is asserted to read the same document differently.Codex finished its Code Review on
d49d09a4with no findings (thumbs-up reaction, no inline comments); the Codex Security Review was skipped by its own usage-limit notice.Recorded oracle answers
internal/runtime/doctor/testdata/hooktrust/identity-oracle.json(generated) holds 68 cases recorded bytestdata/hooktrust/record-identity.mjs(Node v24.20.0) against the v0.2.40 dist: the ten labels; the prototype names; the six refusals with their exact spellings; non-ASCII, U+2028 and lone-surrogate commands; the empty matcher; the timeout spellings (absent, 600, 0, 0.5, 1.5, -3, 1e16, 1e20, 1e21, 9007199254740993, and raw1e400literals); the async and statusMessage shapes. Each case stores its handler as raw JSON text, so a spelling JSON cannot re-encode survives; the recorder is deterministic (re-recording is byte-identical). Cross-checks against the oracle's own pinned goldens reproduce (sha256:5be9da5e...,sha256:84e1bb49...), and the live CRW Stop hook issha256:d919278c....The ported B tests (
hook-trust.test.ts:53-126): the live Stop golden readsplugins/crw/wiring/hooks/stop-recording-completion.jsonand compares it against the recorded case, with a drift assertion on the handler; the SubagentStop matcher golden and the agent-thread PermissionRequest/SessionStart goldens replay the recorded v0.2.40 documents, because CRW declares no such hooks yet; matcher-by-event, timeout default/clamp and statusMessage-presence are Go relational tests. Only the identity half of the third upstream test ports: itslistHookEntries/trust-status half belongs to the hook entry listing and the config.toml trust diagnosis issues.Corpus
No fixture in
contract/fixtures/cxcis driven by this unit alone: thehooks retrustfixtures (cli__hooks__retrust_needs_bootstrap_then_writes,cli__hooks__retrust_safety_pin_refuses_all_drifted) exercise the retrust CLI that a later issue ports, and the doctor fixtures exercisecrw doctor; this PR alone cannot make any of them pass, socontract/notes/cxc/CRW-347.jsonclaims none (the empty-claims convention ofcontract/notes/cxc/CRW-193.json).Size
755 counted lines (added only), generated data excluded: 316 implementation, 316 tests, 117 recorder, 6 documentation.
identity-oracle.json(493 lines) is generated and excluded.Out of scope
Hook entry listing (
listHookEntries), the config.toml trust diagnosis, and retrust, plus every other file; the activation surface (plugins/crw/wiring/hooks,plugins/crw/skills,.codex-plugin) is untouched.