Skip to content

fix(analyze-hook): AI must canonicalize hook name and description#566

Merged
marktoda merged 2 commits into
mainfrom
fix/ai-canonical-metadata
May 21, 2026
Merged

fix(analyze-hook): AI must canonicalize hook name and description#566
marktoda merged 2 commits into
mainfrom
fix/ai-canonical-metadata

Conversation

@marktoda

Copy link
Copy Markdown
Contributor

Summary

Closes a trust gap in the analyze-hook workflow: submitter-provided name and description fields from the GitHub issue form were flowing verbatim into the registry. A submitter could put promotional, false-audit, or affiliation claims (e.g. "Uniswap Official — Audited by Trail of Bits") in those fields and have them land unchanged in hooks/<chain>/<address>.json.

Root cause was two-sided:

  1. scripts/assemble_hook.py priority chain was submitter > claude > contractName — submitter won unconditionally if non-empty.
  2. .claude/prompts/classify-hook.md §6-7 explicitly told the AI to defer to the submitter. So even if you flipped the priority, Claude would obediently echo the submitter's text back in its own structured output. The prompt was load-bearing.

This PR fixes both, plus tightens the second-line PR review and adds a structured warnings channel.

What changed

.claude/prompts/classify-hook.md

  • §6 (name) rewritten as an acceptance gate. AI always derives a baseline name from the verified source (contractName + project-qualified labels). Submitter's suggestion is adopted only if all of: factually consistent with the source, free of promotional / audit / safety / affiliation language ("Official", "Audited", "Safe", "Trusted", brand names not in source), and ≤100 chars. Otherwise AI's text wins.
  • §7 (description) same model. AI drafts from source; submitter version adopted only if every claim is substantiated by the Solidity logic.
  • §8 (new) warnings — structured array surfacing rejected submitter suggestions and getHookPermissions() vs. address-bitmask mismatches. Replaces the prior practice of burying flag discrepancies inside the description (which made them invisible).
  • "Important" section now flags submission.json name/description as untrusted by default.

.github/workflows/analyze-hook.yml

  • Claude's structured-output JSON schema gains a required warnings: string[] field (max 20 entries, 300 chars each).

scripts/assemble_hook.py

  • Submitter removed from canonical name/description chain. New order:
    • name: claude_output["name"]source_meta["contractName"]"UnnamedHook" (last two are defense-in-depth)
    • description: claude_output["description"]""
  • Warnings render as a bullet list in the PR body's Warnings section (still says None when empty).

.claude/prompts/review-hook.md §2d

  • Second-line PR review now applies the same acceptance criteria. Previously said "name should match ContractName or be a reasonable override" — the "reasonable override" wording was too permissive. Now explicitly rejects promotional / audit / safety / affiliation language unless those terms appear in the verified source itself.

scripts/test_assemble_hook.py

  • Tests flipped to assert claude > contractName, submitter text never adopted directly.
  • New tests: test_assemble_claude_name_always_wins_over_submitter, test_assemble_falls_back_to_contract_name_when_claude_empty, test_assemble_description_empty_when_claude_empty, test_generate_pr_body_renders_warnings_as_bullets.

What this does NOT defend against

Subtle / plausibly-correct misframings (e.g. "Liquidity Manager" for a one-pair AMM). The acceptance criteria target unverifiable claims, not nuanced framing. The review-hook.yml second-line check is meant to catch this, and rejected suggestions now appear in the PR body's Warnings section as an audit trail for the human approver. The merge gate is still load-bearing.

Test plan

  • python -m pytest scripts/ — all 48 tests pass locally
  • Inline JSON schema in analyze-hook.yml parses and includes warnings in required
  • Verify in CI: open a test submission issue with a promotional name (e.g. "Uniswap Official Hook") and confirm the AI rejects it, uses ContractName, and adds a warning bullet to the PR body
  • Verify in CI: open a clean submission and confirm warnings section shows None

🤖 Generated with Claude Code

Submitter-provided name and description fields flowed verbatim into the
registry because assemble_hook.py's priority chain was submitter > claude >
contractName, and classify-hook.md explicitly told the AI to defer to the
submitter. This let an issue creator put promotional, false-audit, or
affiliation claims in name/description and have them land in the registry
unchanged.

Changes:
- classify-hook.md §6-7: rewrite as acceptance gates. AI always derives a
  baseline from source; submitter suggestion is adopted only if factually
  consistent with the source and free of promotional / audit / safety /
  affiliation language. Otherwise AI's text wins.
- classify-hook.md §8 (new): structured `warnings` array. Records rejected
  submitter suggestions and flag mismatches. Replaces the prior hack of
  burying flag discrepancies inside the description.
- analyze-hook.yml: structured-output schema gains required
  `warnings: string[]` (max 20 entries, 300 chars each).
- assemble_hook.py: submitter removed from canonical name/description
  chain. Claude is canonical; contractName is defense-in-depth fallback.
  Warnings render as bullets in the PR body's Warnings section.
- review-hook.md §2d: second-line PR review now applies the same
  acceptance criteria so it would reject any promotional name/description
  that slipped past analyze-hook.
- test_assemble_hook.py: tests flipped to assert claude > contractName,
  with submitter text never adopted directly; warnings rendering covered.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@vercel

vercel Bot commented May 21, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
hooklist Ready Ready Preview, Comment May 21, 2026 6:52pm

Request Review

Zizmor's github-app audit flagged the create-github-app-token step as
high-severity because the token inherited blanket installation
permissions. The token is only used by the "Create PR" step (push branch,
gh pr create, gh pr merge --auto), so scope it explicitly to
contents:write and pull-requests:write.

This finding existed before but only surfaced now because the zizmor
workflow only audits files changed in the PR diff, and this PR is the
first one to modify analyze-hook.yml.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@marktoda
marktoda merged commit c3a6579 into main May 21, 2026
8 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