Skip to content
Closed
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 92 additions & 0 deletions .github/workflows/claude-code-review.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
name: Claude Code Review

# Automated PR review powered by Claude Code.
# Auth: OAuth token from a Claude subscription (Pro/Max/Team), NOT an API key.
# Generate it locally with `claude setup-token` and store the value as the
# repository secret CLAUDE_CODE_OAUTH_TOKEN
# (Settings -> Secrets and variables -> Actions -> New repository secret).

on:
pull_request:
types: [opened, synchronize, ready_for_review, reopened]

# Cancel an in-flight review when new commits are pushed to the same PR,
# so we only pay for a review of the latest state.
concurrency:
group: claude-review-${{ github.event.pull_request.number }}
cancel-in-progress: true

jobs:
claude-review:
# Skip drafts β€” review once the PR is marked ready.
if: github.event.pull_request.draft == false
runs-on: ubuntu-latest
permissions:
contents: read
pull-requests: write
id-token: write
steps:
- name: Checkout repository
uses: actions/checkout@v4
with:
fetch-depth: 1

- name: Claude Code Review
uses: anthropics/claude-code-action@v1
with:
claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
# Use the built-in Actions token to post comments, so the Claude
# GitHub App does not need to be installed on the repo. Comments
# appear as github-actions[bot]. (To have them appear as "Claude"
# and re-trigger CI, install https://github.com/apps/claude and
# drop this line.)
github_token: ${{ secrets.GITHUB_TOKEN }}

# Post a tracking comment with progress checkboxes as the review runs.
track_progress: true

prompt: |
REPO: ${{ github.repository }}
PR NUMBER: ${{ github.event.pull_request.number }}

You are reviewing a pull request for CloakPipe β€” a Rust-native
privacy proxy that detects, masks, and unmasks PII in LLM traffic,
plus a cryptographic ledger/verifier (chain, signatures, anchors,
proofs). Correctness and privacy guarantees matter more than style.

Focus your review on:

1. **Privacy / leak safety** (highest priority)
- Any code path where PII could pass through unmasked, be logged,
persisted, or emitted in errors, telemetry, or panics.
- Mask/unmask round-trip correctness and placeholder collisions.
- New entity handling that could regress the zero-leak gates.

2. **Cryptographic integrity**
- Ledger hashing/chaining, signature verification, anchors, and
Merkle proofs β€” verify no check is weakened, skipped, or made
to fail open. Constant-time comparisons where relevant.

3. **Rust correctness**
- `unwrap`/`expect`/`panic!` on paths reachable from untrusted
input; unhandled `Result`/`Option`; integer/slice panics.
- `unsafe` blocks, incorrect lifetimes, and concurrency hazards
(data races, deadlocks, `.await` while holding a lock).
- Error handling that silently swallows failures.

4. **Performance**
- Regressions on the hot masking path (target <5ms); needless
allocations or clones in per-request code.

5. **Testing**
- Adequate coverage for new logic; missing edge cases; whether
cloakleak / verifier gates still hold.

Report concrete issues as inline comments on the exact lines. Use a
top-level comment for the summary and any general observations. Be
specific and cite file:line. Do not flag pure style nits unless they
affect correctness or safety.

# Read-only repo access plus the ability to post review comments.
claude_args: |
--allowedTools "mcp__github_inline_comment__create_inline_comment,Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*)"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟑 Nit: --allowedTools includes Bash(gh pr comment:*) alongside mcp__github_inline_comment__create_inline_comment and track_progress: true. This lets Claude post extra freeform top-level PR comments outside the single tracked/managed comment the rest of this setup relies on, which can lead to duplicate/orphaned comments on re-runs instead of one comment being updated. Consider dropping gh pr comment from the allowlist so all output goes through track_progress + inline comments.

49 changes: 49 additions & 0 deletions .github/workflows/claude.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
name: Claude

# On-demand assistant: responds when you mention @claude in an issue or PR
# comment (e.g. "@claude review this PR", "@claude why does this test fail?").
#
# IMPORTANT: issue_comment / pull_request_review_comment / issues triggers only
# run the workflow version on the DEFAULT branch (main). This file must be
# merged to main before @claude mentions do anything β€” mentions on a PR whose
# base is main use main's copy of this workflow, not the PR branch's.

on:
issue_comment:
types: [created]
pull_request_review_comment:
types: [created]
pull_request_review:
types: [submitted]
issues:
types: [opened, assigned]

jobs:
claude:
# Only run when @claude is actually mentioned.
if: |
(github.event_name == 'issue_comment' && contains(github.event.comment.body, '@claude')) ||
(github.event_name == 'pull_request_review_comment' && contains(github.event.comment.body, '@claude')) ||
(github.event_name == 'pull_request_review' && contains(github.event.review.body, '@claude')) ||
(github.event_name == 'issues' && (contains(github.event.issue.body, '@claude') || contains(github.event.issue.title, '@claude')))
runs-on: ubuntu-latest
permissions:
contents: write # allow @claude to apply and push fixes when asked
pull-requests: write
issues: write
id-token: write
Comment on lines +27 to +45

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

πŸ”΄ Important β€” anyone can trigger a write-privileged agent by commenting "@claude"

The job's if: only checks that the comment/issue/review body contains the string @claude (lines 25-28). It never checks github.event.comment.author_association (or the issue/review equivalents), and the job runs with contents: write, pull-requests: write, and issues: write (lines 31-34).

On a public repo this means any GitHub account β€” not just maintainers or collaborators β€” can open an issue or comment on any PR (including one they opened from a fork) with @claude <arbitrary instructions> and get an agent to run with push access to the repo and issue/PR write access. Since claude.yml doesn't set claude_args/--allowedTools at all, the action's default (fairly broad) tool access applies, so the blast radius of an untrusted instruction here is large β€” pushing unreviewed commits, editing issues/PRs, etc.

Suggest gating the condition on author association, e.g.:

if: |
  (github.event_name == 'issue_comment' && contains(github.event.comment.body, '@claude') &&
   contains(fromJSON('["OWNER","MEMBER","COLLABORATOR"]'), github.event.comment.author_association)) ||
  ...

and the same for the pull_request_review_comment, pull_request_review, and issues branches.

Fix this β†’

steps:
- name: Checkout repository
uses: actions/checkout@v4
with:
fetch-depth: 1

- name: Claude
uses: anthropics/claude-code-action@v1
with:
claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
# Built-in token to post comments (no Claude GitHub App install needed).
github_token: ${{ secrets.GITHUB_TOKEN }}
# No `prompt`: the action replies to whatever you write after @claude.
# Same CloakPipe review focus is available on request, e.g.
# "@claude review this PR for PII leaks and crypto-verification issues".
67 changes: 67 additions & 0 deletions REVIEW.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
# Review instructions

CloakPipe is a Rust-native privacy proxy that detects, masks, and unmasks PII
in LLM traffic, plus a cryptographic ledger and verifier (chain, signatures,
anchors, Merkle proofs). Correctness and privacy guarantees outrank style.

Open the review summary with a one-line tally (e.g. `3 important, 2 nits`), and
lead with "No blocking issues" when nothing is Important.

## What Important (πŸ”΄) means here

Reserve πŸ”΄ for findings that would break a privacy guarantee, corrupt the
verifiable ledger, or crash on untrusted input. Specifically:

- **PII leakage**: any path where raw/unmasked PII can be logged, persisted,
cached, sent to an upstream model, or surfaced in an error, panic message,
span, or metric label. Broken mask/unmask round-trips and placeholder
collisions count here.
- **Weakened crypto verification**: ledger hashing/chaining, signature checks,
anchors, or proof validation that is skipped, made to **fail open**, uses a
non-constant-time comparison for secrets/MACs, or accepts a malformed bundle
that should be rejected.
- **Panic on untrusted input**: `unwrap`/`expect`/`panic!`/`unreachable!`,
slice/index or integer-overflow panics, on a path reachable from request
data or an untrusted bundle.
- **Concurrency hazards**: data races, deadlocks, or holding a lock across an
`.await`.
- **Regressions to the zero-leak gates** (cloakleak) or the verifier e2e checks.

Style, naming, formatting, and refactor suggestions are 🟑 Nit at most.

## Cap the nits

Report at most **five** 🟑 Nits per review. If you found more, add
"plus N similar items" to the summary instead of posting them inline.

## Do not report

- Anything CI already enforces: `cargo fmt`, `cargo clippy -- -D warnings`,
and compiler warnings (see `.github/workflows/ci.yml`).
- `Cargo.lock` and any generated or vendored files.
- The intentionally-insecure **baseline** implementations in cloakleak that are
designed to leak 100% (they exist as a reference β€” do not flag them as leaks).
- Test fixtures and corpora that contain fake/sample PII on purpose.

## Always check

- New PII entity detectors / maskers ship with tests, and any change is
reflected in the cloakleak tracks (prose + tool_json) so the zero-leak gate
still passes.
- Log lines, error messages, panics, and telemetry never include raw PII,
secrets, or full request/response bodies.
- Verifier checks (`chain`, `sigs`, `anchors`, `proofs`) fail **closed** β€” a
missing, malformed, or tampered field must be an error, never a silent pass.
- New dependencies are justified; watch for anything pulling PII or ledger data
off-box.

## Verification bar

Behavior claims need a `file:line` citation in the source, not an inference
from a name. If you cannot point to the code that causes the issue, downgrade
to a question in the summary rather than a πŸ”΄ inline comment.

## Re-review convergence

After the first review of a PR, suppress new nits and post πŸ”΄ Important
findings only. Don't re-raise items already fixed.
Loading