Skip to content

CRW-347: port the hook trust identity hash - #560

Merged
thisisjun786 merged 5 commits into
devfrom
codex/crw-347-hooktrust-identity
Oct 5, 2026
Merged

thisisjun786 merged 5 commits into
devfrom
codex/crw-347-hooktrust-identity

Conversation

@thisisjun786

@thisisjun786 thisisjun786 commented Oct 5, 2026 •

Copy link
Copy Markdown
Owner

What this changes

internal/runtime/doctor/hooktrust_identity.go adds HookTrustIdentityHash(event string, matcher *string, handler map[string]any) (string, error), the read-only port of CXC v0.2.40 hook-trust.ts:18-38 (EVENT_LABELS, MATCHER_DROPPED_EVENTS), :40-47 (HookHandler) and :96-139 (assertSupportedPlatform, sorted, identityHash) at commit 3c1459ac. It returns sha256: 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:

  • the ten event labels;
  • the six refusal messages verbatim (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;
  • the handler normalizes to {type: "command", command, timeout: max(timeout or 600, 1), async: async or false}, with statusMessage only when it is not null;
  • the matcher is dropped for UserPromptSubmit and Stop only, an empty matcher included;
  • the canonical JSON has recursively sorted keys and is written by internal/pyjson.Dumps with Compact, SortKeys and Unicode; the clamped timeout is pre-spelled the way JavaScript's JSON.stringify prints a number (600, 1.5, 1e+21, 10000000000000000) and handed to Dumps as a json.Number, because pyjson.Float is Python repr (600.0).
  • a json.Number past the double range reads as the infinity JavaScript holds, so a refused type of 1e400 refuses as unsupported 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 JavaScript Object.prototype member is accepted (the eleven function members hash without event_name, __proto__ with event_name: {}), and an object type with an own toString member makes the type refusal throw the raw TypeError Cannot convert object to primitive value.

Criteria

  • port parity: the 68 recorded oracle answers replay through go test, and the six ported B tests stand as Go tests;
  • tests: red first (commit 481075124: 78 assertion failures with the stub), then green at the head;
  • corpus: no contract/fixtures/cxc fixture is driven by this unit alone, so contract/notes/cxc/CRW-347.json claims none;
  • invariants: no existing contract, golden or test is edited, and the activation surface is untouched;
  • parity and defects: the two kept lines above;
  • runtime: no init, no package-level initializer doing work, no Node;
  • size: 755 counted lines (generated data excluded), stated below;
  • delivery: this PR, every CI job on one head, and the two external reviews waited for.

Verification

Red first: commit 481075124 holds the tests plus a stub, and go test -count=1 ./internal/runtime/doctor fails 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.332s
  • CGO_ENABLED=0 go build ./... and go vet ./internal/runtime/doctor - exit 0; go vet ./... and GOOS=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 below
  • crw-dev ci validate, crw-dev ci contracts - exit 0
  • crw-dev ci plugin - Package 0.4.0+adfc99ffc96f ...: 216 files, digest 635105c68ea07f89, the same version and digest the base 5c929a6e14 prints, and no working-tree drift line: the activation surface is untouched
  • gofmt -l internal/runtime/doctor empty; git diff --check origin/dev...HEAD empty
  • the pinned gitleaks over origin/dev..HEAD (3 commits): no leaks
  • hosted CI: CRW CI run 37253119101, dispatched on this exact head because the pull request conflicts only in the append-only docs/port-cxc/known-defects.md (GitHub starts no pull_request run for that); every job must be green on it before the handoff

Review round

Devin reviewed d49d09a4 once and found two yellow items, both fixed in bcea10a33 and answered and resolved on their threads:

  • an overflowed type spelling (1e400) fell through to %v instead of the oracle's Infinity: the range-error value is now kept, -1e400 refuses as -Infinity, and three recorded cases pin it;
  • a lone surrogate in a command hashes U+FFFD after encoding/json reads it, which is a reader boundary, not something the unit can recover: the doc comment now names pyjson.Loads with LoadOptions{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 d49d09a4 with 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 by testdata/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 raw 1e400 literals); 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 is sha256:d919278c....

The ported B tests (hook-trust.test.ts:53-126): the live Stop golden reads plugins/crw/wiring/hooks/stop-recording-completion.json and 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: its listHookEntries/trust-status half belongs to the hook entry listing and the config.toml trust diagnosis issues.

Corpus

No fixture in contract/fixtures/cxc is driven by this unit alone: the hooks retrust fixtures (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 exercise crw doctor; this PR alone cannot make any of them pass, so contract/notes/cxc/CRW-347.json claims none (the empty-claims convention of contract/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.


Devin Review

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.
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for security reviews. Please try again later.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-05T01:50:44.936346Z d49d09a PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 potential issues.

Devin Review

Comment thread internal/runtime/doctor/hooktrust_identity.go
Comment thread internal/runtime/doctor/hooktrust_identity.go
…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.
@thisisjun786
thisisjun786 merged commit 270a056 into dev Oct 5, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant