Skip to content

examples: address external review finding fixes#4

Merged
heskew merged 1 commit intomainfrom
examples/finding-fixes
Apr 24, 2026
Merged

examples: address external review finding fixes#4
heskew merged 1 commit intomainfrom
examples/finding-fixes

Conversation

@heskew
Copy link
Copy Markdown
Member

@heskew heskew commented Apr 24, 2026

Summary

Mirror of HarperFast/harper#402's fae521c commit. Applying the finding-fix pass from the deep external review to the public example workflows so consumers pin against a version that carries the calibrated shape.

Fixed

  • docs: USAGE.md + refresh README Security section #5 label match: explicit whitelist (claude-fix:typo|docs|deps|bug) instead of startsWith('claude-fix:'), with a comment explaining why prefix matching admits typoed variants.
  • #6 heredoc marker: random EOF_$(openssl rand -hex 16) delimiter in examples/claude-review.yml's compose step.
  • #7a eager npm ci on mention: removed. Comment on Setup Node.js explains why — most mentions don't need deps.
  • #8a mention model: Sonnet 4.6 default with Opus 4.7 opt-in via case-insensitive word-boundary deep in the comment body. Cost gets spent deliberately.

Mention-parsing step (new in this PR for the example)

New shell step that enforces @claude as the first non-whitespace token. Subsequent steps guard on steps.mention.outputs.proceed == 'true' so non-matching comments exit cleanly without running the agent. Also outputs the model name per the deep word-boundary match.

Comment sharpening

🤖 Generated with Claude Code

Same finding-fix pass applied to the public example workflows so
consumers pin against a version that carries the calibrated shape.

- #5 (label match too permissive): Tighten the example's issue-to-PR
  `if:` from `startsWith('claude-fix:')` to an explicit whitelist,
  with a comment calling out why prefix matching is a trap.
- #6 (heredoc collision): Random `EOF_$(openssl rand -hex 16)`
  delimiter in the compose step.
- #7a (eager `npm ci` on mention): Drop the `Install dependencies`
  step. Comment on Setup Node explains why; prompt can tell the agent
  to install before scripts that need deps.
- #8a (Opus cost): Sonnet default with Opus opt-in via case-insensitive
  word-boundary `deep` anywhere in the comment body.
- #9 (scope-to-diff): Already addressed in previous PR (#2) — no change
  needed here.

Plus the mention-parsing step enforcing `@claude` as first non-whitespace
token (word-boundary after), with `steps.mention.outputs.proceed` gating
subsequent steps.

Comment sharpening:

- #1 (postinstall RCE via package.json edit): Example's allowlist
  comment now names this path explicitly and directs consumers to
  `.npmrc` `ignore-scripts=true` as the npm-config-level mitigation.
- #2 (`Bash(git:*)` rationale): review.yml comment now says "read-only
  by design" instead of implying it's universal guidance.

Consumer-facing note: everything marked TODO: pin still needs the
consumer to substitute their own pinned SHAs. Nothing security-critical
here changes that expectation.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@heskew heskew merged commit 7bac35b into main Apr 24, 2026
@heskew heskew deleted the examples/finding-fixes branch April 24, 2026 20:53
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