Skip to content

fix: merge latest dev updates into main#17

Merged
nadav-node9 merged 12 commits intomainfrom
dev
Mar 16, 2026
Merged

fix: merge latest dev updates into main#17
nadav-node9 merged 12 commits intomainfrom
dev

Conversation

@nadav-node9
Copy link
Contributor

Auto-generated PR

Merge latest dev changes into main to trigger a release.

⚠️ Important: When you click Squash and Merge, ensure the commit message starts with:

  • fix: to publish a Patch release (0.0.X)
  • feat: to publish a Minor release (0.X.0)
    If it starts with chore:, no NPM package will be published!

@nadav-node9
Copy link
Contributor Author

🤖 Claude Code Review

Review

Critical Security Issue ⚠️

The AI review script must not run on PRs that modify itself — this creates a trivial bypass where an attacker can:

  1. Submit a PR that disables the review or changes the prompt
  2. The compromised script reviews itself and approves

Required fix: Add a path filter to .github/workflows/ai-review.yml:

on:
  pull_request:
    branches: [main]
    paths-ignore:
      - '.github/workflows/ai-review.yml'
      - 'scripts/ai-review.mjs'

Other Issues:

  1. Missing GITHUB_REPOSITORY env var: The workflow doesn't set it. GitHub Actions provides it automatically, but should validate it exists before the split('/') call to avoid silent failures.

  2. Secrets exposure risk: AUTO_PR_TOKEN needs write access to post comments. Document required scopes (repo or public_repo + write:discussion). If this token has broader permissions, a compromised dependency in the npm install step could exfiltrate it.

  3. Diff truncation loses context: Truncating at 20k chars mid-line could break syntax and produce nonsensical reviews. Consider truncating at file boundaries or adding a warning comment when truncation occurs.

  4. No rate limiting: If many PRs are opened quickly, this will hit Anthropic API rate limits and fail silently. Consider adding retry logic or batching.

  5. License change buried in README: Apache 2.0 → this is a legal change, not a docs fix. Should be in a separate commit with justification. Verify all contributors have agreed to relicense.

  6. Merge conflict artifacts in README: The diff shows Git conflict markers were resolved but duplicated content was removed. Good cleanup, but verify no actual content was lost.


Minor:

  • Model name claude-sonnet-4-5 — verify this is correct (not claude-sonnet-4-20250514).
  • max_tokens: 1024 might be tight for 400-word reviews; consider 1500-2000.
  • The process.exit(1) on error will mark the workflow as failed — intended behavior, but document it.

Verdict: Block until self-modification bypass is fixed. The license change also needs separate discussion.


Automated review by Claude Sonnet

- native.ts: add extractContext + formatArgs with matchedField/matchedWord
  tracing for "Context Sniper" popup — shows dangerous word in context
- core.ts: extend evaluatePolicy return with matchedField/matchedWord;
  per-field scan after dangerous word found; pass through authorizeHeadless
- daemon/index.ts: gate SSE broadcast and browser open on browser config flag
- LICENSE/package.json/README.md: MIT → Apache-2.0
- .github/workflows/ai-review.yml: add paths-ignore to prevent self-modification
- scripts/ai-review.mjs: upgrade to claude-sonnet-4-6, max_tokens 2048

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@nadav-node9
Copy link
Contributor Author

🤖 Claude Code Review

Code Review

License Change (MIT → Apache 2.0)

Potential legal issue: This is a breaking change for any downstream users who took a dependency under MIT. Apache 2.0 adds patent termination clauses and attribution requirements that weren't present before. The package.json, LICENSE, and README badge are updated consistently, but there's no NOTICE file (required by Apache 2.0 Section 4(d) if the original work had one). Minor, but worth confirming intentionality with legal/stakeholders before merging.

README Cleanup

The removal of the duplicate "Node9 keeps the last 10 snapshots" paragraph and the merge conflict artifacts (<<<<<<< Updated upstream, =======, >>>>>>> Stashed changes) are correct and necessary. Someone committed with unresolved merge conflicts — that should never have reached a PR. Check your pre-commit hooks.

AI Review Workflow (ai-review.yml + ai-review.mjs)

This is the most significant addition and has several concerns:

Security — Token Scope:
AUTO_PR_TOKEN is used to post comments back to GitHub. Verify this token has only pull-requests: write and contents: read. If it's a broad PAT, a compromised workflow could escalate. Prefer a fine-grained token.

Security — Prompt Injection:
The diff content is passed directly into the Claude prompt with no sanitization. A malicious PR author could craft diff content to manipulate the AI reviewer's output (e.g., inject instructions into a string literal in source code). For a security tool, this is ironic and worth addressing — at minimum, clearly delimit the diff section in the prompt.

Silent truncation:
MAX_DIFF_CHARS = 20000 silently truncates large diffs. The review comment won't mention truncation, so reviewers may get a false sense of complete coverage. Add a note in the posted comment when truncation occurs.

Error handling:
process.exit(1) on failure will mark the CI check as failed. That's intentional, but if the Anthropic API is down, it blocks all PRs. Consider process.exit(0) with a warning comment instead, or make the workflow step continue-on-error: true.

paths-ignore self-exclusion:
The workflow excludes changes to ai-review.yml and scripts/ai-review.mjs itself from triggering review. This means changes to the review script are never reviewed by the AI reviewer. Fine for bootstrapping, but be aware of the blind spot.

Missing --frozen-lockfile:

run: npm install @anthropic-ai/sdk @octokit/rest

This installs unpinned latest versions at CI time. Use exact versions or a lockfile to prevent supply-chain drift. For a security-focused project this is especially important.

parseInt without validation:
parseInt(process.env.PR_NUMBER)parseInt returns NaN on bad input, but the !prNumber check catches NaN (falsy). This is fine, but a comment would clarify intent.

Summary

The substantive concerns are: (1) prompt injection in the AI reviewer, (2) unpinned CI dependencies, and (3) the license change deserving explicit sign-off. The README cleanup is straightforward and correct.


Automated review by Claude Sonnet

- Wrap diff in <diff>...</diff> markers with untrusted-content notice to mitigate prompt injection
- Surface truncation note in posted PR comment when diff exceeds MAX_DIFF_CHARS
- Downgrade API errors to warning comments + exit 0 so Anthropic outages don't block PRs
- Pin @anthropic-ai/sdk@0.78.0 and @octokit/rest@22.0.1 to prevent supply-chain drift
- Add explicit permissions block (contents: read, pull-requests: write)
- Exclude dependabot[bot] from triggering review
- Add fetch-depth: 0 to checkout step

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@nadav-node9
Copy link
Contributor Author

🤖 Claude Code Review

Code Review

README.md

The diff cleanly resolves a merge conflict (removes duplicated CLI reference section and the stray <<<<<<< Updated upstream / ======= / >>>>>>> Stashed changes markers) and removes a duplicated paragraph about snapshots. These are straightforward cleanup — no issues.

LICENSE + package.json

MIT → Apache 2.0 relicense. The change is internally consistent (LICENSE text, badge, package.json all updated). One concern: Apache 2.0 has different downstream implications than MIT (patent termination clause, NOTICE file requirements). If any dependencies are MIT-only and incompatible, or if users have already taken MIT-licensed copies, this warrants a separate legal review. Not a code bug, but worth flagging explicitly in the PR description.

.github/workflows/ai-review.yml

  • AUTO_PR_TOKEN scope: The workflow uses a PAT (AUTO_PR_TOKEN) with pull-requests: write. Verify this token is scoped to only that permission and not a broad repo token — a compromised secret here would allow writing to PRs across the repo.
  • paths-ignore bypass risk: The exclusion of ai-review.yml and scripts/ai-review.mjs from self-review is a sound design choice to prevent prompt injection, and the comment explains it well. However, changes to those files still run the previous workflow version, so a malicious actor with push access could stage a two-step attack. Acceptable tradeoff for a CI script, but worth documenting.
  • fetch-depth: 0: Full history checkout for a diff-reading script is heavier than needed. fetch-depth: 2 would suffice for PR diffs and would be faster.
  • Pinned npm install without lockfile: Running bare npm install with pinned semver strings (not SHAs) on each run bypasses the repo lockfile and fetches from the registry on every run. Use npm ci with a committed lockfile, or at minimum add --ignore-scripts to reduce supply-chain risk.

scripts/ai-review.mjs

The diff is truncated — the file is cut off at the repo/owner destructuring. This makes it impossible to review the actual review logic, prompt construction, or how the diff is passed to Claude. This is a security-critical gap: the prompt passed to the Anthropic API is the primary injection surface, and it's not visible in this diff. The full file must be included before merging.

Summary

  • Merge conflict cleanup and license update look fine.
  • Block on: full ai-review.mjs content must be reviewed (truncated diff).
  • Fix: pin workflow deps via lockfile + npm ci.
  • Minor: reduce fetch-depth, verify PAT scope.

⚠️ Note: This diff exceeded 20,000 characters and was truncated. The review above covers only the first portion of the changes.


Automated review by Claude Sonnet

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@nadav-node9
Copy link
Contributor Author

🤖 Claude Code Review

Code Review

README.md Cleanup

The merge conflict artifacts (<<<<<<< Updated upstream, =======, >>>>>>> Stashed changes) and duplicate sections are correctly removed. The duplicate "keeps the last 10 snapshots" paragraph removal is good. No issues here.

License Change (MIT → Apache 2.0)

The LICENSE file, package.json, and README badge are all consistently updated. No inconsistencies spotted. One note: downstream users who pinned to the MIT license should be notified — this is a more restrictive change for certain use cases. Consider a CHANGELOG entry or release note.

ai-review.yml — Security Concerns (Several Issues)

1. Prompt injection risk — this is the most serious issue.
The workflow feeds PR diff content directly to Claude as part of the review prompt (visible in scripts/ai-review.mjs). A malicious PR author can embed instructions in their diff to manipulate the AI reviewer's output — e.g., to post a false "LGTM" comment. The paths-ignore exclusion of the review script itself is a good mitigation for self-modification, but it doesn't address prompt injection in arbitrary diff content. The system prompt must strictly instruct Claude to treat diff content as untrusted data, and the review comment should clearly label it as AI-generated and non-authoritative.

2. AUTO_PR_TOKEN scope is unclear.
Using a PAT (AUTO_PR_TOKEN) instead of the default GITHUB_TOKEN for posting comments is unnecessary here — GITHUB_TOKEN with pull-requests: write is sufficient and scoped to the repo. A PAT may have org-wide write access, which violates least privilege.

3. Unpinned npm install at CI time.
Dependencies are version-pinned (@anthropic-ai/sdk@0.78.0, @octokit/rest@22.0.1), which is good. However, they're installed without a lockfile, meaning transitive dependencies are unpinned and could drift. Use a lockfile or --ignore-scripts at minimum.

4. scripts/ai-review.mjs is truncated in the diff.
The file cuts off mid-line at const [repoOwner, re — this means the full script isn't reviewable. The most security-critical part (how the diff is passed to Claude, what the system prompt says, whether output is sanitized before posting) is not visible. This PR should not be merged until the complete file is included in the diff.

5. if: condition doesn't cover all bots.
The actor exclusion only covers github-actions[bot] and dependabot[bot]. Other automation actors (e.g., Renovate, release bots) would still trigger an AI review and incur API costs.

Summary

Two blockers: the truncated ai-review.mjs (can't fully assess the security-critical prompt construction) and the PAT token unnecessarily broadening permissions. The prompt injection risk needs explicit mitigation documented or implemented before this ships in a security tool.

⚠️ Note: This diff exceeded 20,000 characters and was truncated. The review above covers only the first portion of the changes.


Automated review by Claude Sonnet

… main

- ai-review.yml: replace AUTO_PR_TOKEN with GITHUB_TOKEN (permissions block
  already scopes it correctly — no broad PAT needed)
- ai-review.yml: add --ignore-scripts to npm install to block malicious
  postinstall hooks from transitive dependencies
- sync-dev.yml: new workflow — after every push to main, merge main back into
  dev so release-bot version bumps don't cause recurring README conflicts on
  the next dev -> main PR

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions
Copy link
Contributor

🤖 Claude Code Review

Review

What this PR does

  • Adds two GitHub Actions workflows: an AI-powered PR review and a devmain sync
  • Migrates the license from MIT to Apache 2.0
  • Cleans up a merge conflict artifact and a duplicate paragraph from README.md

Issues Found

1. sync-dev.ymlcontents: write is overly broad
The workflow checks out dev and force-pushes using AUTO_PR_TOKEN (a PAT), but the permissions block grants contents: write at the job level with no further restriction. A compromised or malicious commit message that doesn't contain [skip ci] will trigger a merge into dev automatically. Consider adding branch protection on dev requiring PR reviews, or restricting this workflow to only run from trusted actors.

2. sync-dev.yml[skip ci] commit-message check is bypassable
The skip condition is !contains(github.event.head_commit.message, '[skip ci]'). A commit message like fix: real change [skip ci] injected would skip the workflow. That's acceptable for the loop-prevention purpose here, but it's worth noting this is a weak guard — any commit can opt out of the sync.

3. ai-review.ymlnpm install --ignore-scripts without a lockfile
Installing @anthropic-ai/sdk@0.78.0 and @octokit/rest@22.0.1 by semver at runtime means transitive dependencies are resolved fresh on every run. A supply-chain attack on a transitive dep would silently compromise the review step. Use a checked-in package-lock.json or npm ci with a lockfile, or use --frozen-lockfile. The --ignore-scripts flag only blocks direct postinstall hooks, not transitive ones unless propagated.

4. ai-review.yml — The review script (scripts/ai-review.mjs) is not in this diff
The workflow runs node scripts/ai-review.mjs, but the script itself isn't shown. This is the highest-risk surface: it receives the ANTHROPIC_API_KEY and GITHUB_TOKEN, processes untrusted PR diff content, and posts comments. Without reviewing that script, correctness and prompt-injection resistance cannot be verified. This is a blocker for a security tool's CI pipeline.

5. License migration — no contributor acknowledgment
Changing from MIT to Apache 2.0 is a relicensing event. If there are prior contributors, their consent is typically required. This isn't a code bug, but it's a legal/process flag worth raising before merging.

6. README — merge conflict residue removal
The removal of the <<<<<<< Updated upstream / ======= / >>>>>>> Stashed changes conflict markers is correct and necessary. The duplicate CLI reference section cleanup looks good.


No issues with

  • Workflow permissions scoping (pull-requests: write, contents: read) in ai-review.yml — minimal and correct
  • The dependabot[bot] / github-actions[bot] skip guard — appropriate
  • README badge update to Apache 2.0 — consistent with license change

Summary

Block on: item 4 (the review script is unreviewed). Items 1 and 3 are medium-severity concerns worth addressing before merge.

⚠️ Note: This diff exceeded 20,000 characters and was truncated. The review above covers only the first portion of the changes.


Automated review by Claude Sonnet

Move @anthropic-ai/sdk and @octokit/rest into devDependencies and switch the
ai-review workflow from bare npm install to npm ci --ignore-scripts. This
locks all transitive dependencies to the committed lockfile, eliminating
supply-chain drift on every CI run.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions
Copy link
Contributor

🤖 Claude Code Review

Review

What this PR does

  • Adds two GitHub Actions workflows (ai-review.yml, sync-dev.yml)
  • Migrates license from MIT → Apache 2.0
  • Cleans up README: removes a duplicate paragraph, resolves a leftover merge conflict artifact

Issues Found

Critical — Merge conflict debris in README

The diff removes a large block of content that is clearly unresolved merge conflict markers (<<<<<<< Updated upstream, =======, >>>>>>> Stashed changes) along with duplicated CLI reference sections. This is the right cleanup, but it's a red flag that these were ever committed to the branch. Verify the final README renders correctly end-to-end — a truncated node9 explain section at the bottom (n followed by nothing) suggests the resolution may be incomplete. The explain subsection appears to have lost its body content.

sync-dev.yml — PAT with contents: write is overprivileged

AUTO_PR_TOKEN is a Personal Access Token used to push to dev. The permissions: contents: write block sets the GITHUB_TOKEN permissions but the actual push uses the PAT, so the permissions block provides false assurance. If AUTO_PR_TOKEN is a classic PAT with broad repo scope, a compromise of that secret gives full write access to the repo. Document the minimum required scopes for AUTO_PR_TOKEN, and prefer a fine-grained PAT scoped only to this repository.

sync-dev.yml[skip ci] loop prevention is fragile

The guard !contains(github.event.head_commit.message, '[skip ci]') only checks the triggering commit message. If main receives a commit without [skip ci] after the bot's own push, the workflow could still re-trigger on legitimate main pushes in rapid succession. This isn't a security issue but could cause noisy failures or redundant syncs.

ai-review.ymlpaths-ignore self-exclusion is documented but subtle

The comment correctly explains why ai-review.yml and scripts/ai-review.mjs are excluded from self-review. However, this means a malicious PR could modify scripts/ai-review.mjs (e.g., exfiltrate ANTHROPIC_API_KEY or GITHUB_TOKEN) and the AI review job would still run on that modified script. Consider pinning or hashing the review script, or running it from a protected ref rather than the PR's checked-out code.

ai-review.ymlnpm ci --ignore-scripts is correct

Good call using --ignore-scripts. No issue here.

License change

MIT → Apache 2.0 is a meaningful license change with downstream implications for users and contributors. Ensure this is intentional and that any existing contributors have been consulted or that prior contributions were sole-authored. Apache 2.0 adds patent termination clauses which MIT lacks.


Summary

The README cleanup is straightforwardly correct (minus the possible truncated explain section). The main concerns are: (1) the AUTO_PR_TOKEN PAT scope documentation and privilege surface, (2) the AI review script running from untrusted PR code with access to secrets, and (3) confirming the node9 explain docs aren't accidentally missing their body.

⚠️ Note: This diff exceeded 20,000 characters and was truncated. The review above covers only the first portion of the changes.


Automated review by Claude Sonnet

@nadav-node9 nadav-node9 merged commit e809be6 into main Mar 16, 2026
8 checks passed
nadav-node9 pushed a commit that referenced this pull request Mar 16, 2026
## [1.0.8](v1.0.7...v1.0.8) (2026-03-16)

### Bug Fixes

* merge latest dev updates into main ([#17](#17)) ([e809be6](e809be6))
@nadav-node9
Copy link
Contributor Author

🎉 This PR is included in version 1.0.8 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants