fix: multiple improvements for gh workflows#6306
Conversation
ReviewGood security hardening for the One item to confirmRemoving Minor noteThe scoped No bugs or security issues found. LGTM otherwise. |
|
perhaps also revert a018e0d |
|
@kevinjqliu Right, I'm considering seriously. Thank you! |
esteban
force-pushed
the
fix/claude-code-review-security
branch
from
42a0338 to
8cced49
Compare
|
yes! and please rotate the github secrets (in case its already compromised) |
westonpace
left a comment
westonpace
left a comment
There was a problem hiding this comment.
Some minor suggestions but seems like a good cautious step
| uses: anthropics/claude-code-action@26ec041249acb0a944c0a47b6c0c13f05dbc5b44 # v1 | ||
| uses: anthropics/claude-code-action@673eb13aa77026be5c507eda12322c1a58b80f0b # v1 | ||
| env: | ||
| CLAUDE_CODE_SUBPROCESS_ENV_SCRUB: '1' |
There was a problem hiding this comment.
Can you add a comment explaining what this does?
There was a problem hiding this comment.
Updated the right version, hash should be 3ac52d0da9f8ec9ca7b4dc23bb477e36ef9c77a9 for 1.0.79: anthropics/claude-code-action@3ac52d0
There was a problem hiding this comment.
Oh, I meant, what is CLAUDE_CODE_SUBPROCESS_ENV_SCRUB? I don't want someone turning it off in the future.
| # or https://code.claude.com/docs/en/cli-reference for available options | ||
| claude_args: | | ||
| --allowed-tools "Bash(gh issue view:*),Bash(gh search:*),Bash(gh issue list:*),Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*),Bash(gh pr list:*)" | ||
| --allowed-tools "Bash(gh pr comment ${{ github.event.pull_request.number }}:*),Bash(gh pr diff ${{ github.event.pull_request.number }}:*),Bash(gh pr view ${{ github.event.pull_request.number }}:*)" |
There was a problem hiding this comment.
Hmm, restricting comment seems fine. The rest are all read-only and it's a public repo right? Would these be a problem?
That being said I don't really see why it would need to see other PRs or issues. I guess the only case I can think of is when a PR is built for an issue and references back to it (e.g. "closes 5472 see issue for rationale")
There was a problem hiding this comment.
The risk is that other PRs/comments could be used for prompt injection. For sure this limits the value of Claude Code to a small context of the PR. 🤷🏻♂️
wjones127
left a comment
wjones127
left a comment
There was a problem hiding this comment.
IIUC, this will mean reviews won't work for anyone, unless they made the PR from a branch within the repo. Is that correct?
If that's the case, I'm wondering if we should just remove this job and find a different way to run reviews that is more secure. What do you think?
esteban
force-pushed
the
fix/claude-code-review-security
branch
from
8cced49 to
2163b67
Compare
|
please run |
Co-authored-by: Esteban Gutierrez <esteban@lancedb.com>
5 participants
No description provided.