feat: add secret-safe reviewer adapter

[Pigbibi]

Summary

• add the R2 trust-boundary adapter from untrusted structured reviewer output to R1 structured_tokens.v2
• enforce strict schema/bounds, safe credential mapping/redaction, atomic fail-closed errors, and legacy-unverified behavior
• keep R1, runtime adoption, persistence, evidence, and state-machine behavior unchanged

Validation

• python3 -m unittest tests.test_secret_safe_reviewer_adapter_r2 tests.test_canonical_typed_identity_r1d (11 passed)
• python3 -m unittest tests.test_run_codex_pr_review (54 passed)
• python3 -m unittest discover -s tests (631 passed, 1 skipped)
• ruff, compileall, actionlint, and git diff --check passed

Scope

• 2 new files, 349 insertions
• no production dependency or runtime call-site change

[github-actions]

🤖 Codex PR Review

🚫 Merge blocked: 3 serious issue(s) found in high-risk files

⚖️ Codex Review Arbitration

🚫 block: block is required because at least two current findings are directly proven by the current source. First, in scripts/secret_safe_reviewer_adapter.py, SHAPES accepts ghp_, gho_, ghu_, and ghr_ full tokens, but MARKER only flags ghs_ and github_pat_; _display_text() redacts only full PROSE_SHAPES matches and otherwise only rejects MARKER, so partial strings like ghp_partialtoken pass through unchanged. That is the same required behavior as the prior GitHub-token blocker, not a contract conflict. Second, scripts/canonical_typed_identity.py defines SECRET_TYPES = {"credential", "authorization", "api_key", "session_token", "private_key"}, but _map_secret() in the adapter can derive only credential, api_key, or authorization from material because SHAPES has no classifiers for session_token or private_key; any R1-valid payload using those types is therefore rejected with ambiguous_credential_material or credential_type_mismatch, so the adapter is narrower than the R1 contract it claims to accept. The prior logic blocker about validating untrusted structured_tokens before trust gating is fixed in this diff, but that does not contradict any current finding.

🚫 Blocking Issues

These issues must be fixed before this PR can be merged:

1. 🟠 [HIGH] Security in scripts/secret_safe_reviewer_adapter.py

MARKER only treats ghs_ and github_pat_ as ambiguous GitHub-token prefixes, even though SHAPES also accepts ghp_, gho_, ghu_, and ghr_. As a result, prose like ghp_partialtoken or ghr_truncated is neither redacted nor rejected, so token fragments can flow into display and then into comments/artifacts. (line 24)

Suggestion: Build the ambiguous-prefix detector from the same GitHub token families as SHAPES, or explicitly add ghp_, gho_, ghu_, and ghr_ to MARKER so partial GitHub credentials fail closed too.

2. 🟠 [HIGH] Logic in scripts/secret_safe_reviewer_adapter.py

The adapter can only derive credential, api_key, and authorization from secret_ref.material, but R1's canonicalizer accepts additional secret types (session_token and private_key). Any reviewer output that legitimately uses those R1-supported types will be rejected here with ambiguous_credential_material or credential_type_mismatch, which is an API-compatibility break between the new adapter and the existing R1 contract. (line 154)

Suggestion: Either extend the adapter with classifiers for the remaining R1 secret types, or explicitly narrow the accepted reviewer-output schema and tests so callers cannot produce R1-valid types that this layer rejects.

3. 🔴 [CRITICAL] Security in scripts/secret_safe_reviewer_adapter.py

The redaction set only covers a handful of token shapes (gh*, github_pat_, AKIA/ASIA, JWTs, and sk-). Common credential formats already treated as sensitive elsewhere in this bridge, such as GitLab PATs, Slack tokens, or database URLs with embedded passwords, will pass through _display_text unchanged. Because display is what downstream code persists and publishes, this can write raw secrets into PR comments and evidence artifacts. (line 15)

Suggestion: Reuse the repository's broader secret-scrubbing logic for summary/description/suggestion/file, or centralize one shared credential detector so this adapter does not have a narrower leak surface than the existing publication path.

---

Review by Codex PR Review bot • PR

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c223d8358b

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[Pigbibi]

Remediation batch complete at 6654f6d:

• expanded high-confidence GitHub token coverage and blocked embedded complete credential shapes
• removed untrusted unknown keys from error paths
• decides legacy/mixed trust eligibility before parsing structured payloads
• retained the frozen display/identity separation; scope binding remains an R5 runtime-adoption concern

Validation: focused 11 passed; review tests 54 passed; full 631 passed (1 skipped); ruff, compileall, actionlint, and diff-check passed. This was the single allowed remediation batch.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6654f6d274

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Redact attacker-controlled keys from error paths

The existing top-level unknown-field redaction does not cover nested keys: _pointer still preserves raw key text, and callers pass attacker-controlled JSON keys from _scan and _map_node. For example, a reviewer_output.v2 payload with an extra structured_tokens.scope key named ghp_<token> whose child is an invalid surrogate raises invalid_utf8_or_control_character at /findings/0/structured_tokens/scope/ghp_..., leaking the credential before schema validation can replace it with <unknown>.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Reuse existing credential scrub patterns for display

The display redaction loop only applies PROSE_SHAPES, which excludes credential formats the existing review pipeline already treats as sensitive, such as glpat-... tokens and password-bearing DSNs covered by tests/test_run_codex_pr_review.py:253. When reviewer summary or description text contains one of those values, _display_text returns it unchanged in display, so this secret-safe adapter can re-expose credentials that the current finding-history sanitizer suppresses.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Bound parsed findings before full traversal

When callers pass an already-parsed reviewer object, which this public function accepts and the tests exercise, this unconditional recursive scan runs before the MAX_FINDINGS check and before the compact-size check can fail the payload. A malicious parsed payload with a huge findings list therefore forces traversal and serialization of every child instead of failing at 32 findings, turning the adapter's size limits into a CPU/memory DoS only for dict/list inputs.

Useful? React with 👍 / 👎.