justinmclean commented on PR #4244: URL: https://github.com/apache/iggy/pull/4244#issuecomment-5756318434
I think we can make this simpler and safer by never running the PR's code. At the moment, the agent step holds the DeepSeek key, runs with --dangerously-skip-permissions, and is told to build and test the PR head. A fork PR with a malicious build.rs would get the key as soon as a committer types /skill. Reading isn't safe either: serena drives rust-analyzer, which runs build scripts and proc macros by default. A lot of the workflow exists to work around this: the control file snapshot, restoring .claude, .agents and .serena, restoring AGENTS.md at any depth, and the warning that the artifact may hold the key. Much of that goes away if the reviewer only reads. Suggested changes: 1. Replace --dangerously-skip-permissions with an --allowedTools list of read-only tools: Read, Grep, Glob and serena's find and overview tools. No Bash. The prompt also points the agent at replace_symbol_body, which a reviewer shouldn't need. 2. Turn off build scripts and proc macros in rust-analyzer. 3. Drop the build and test path. The CI status file already tells the agent what passed. Without the Rust setup and builds, a run should take a few minutes. 4. Drop rtk and the serena hooks, and move the Simple English rules into comment-style.md instead of loading the plugin. rtk only rewrites Bash calls, so it does nothing once there's no Bash. Only the plugin's SessionStart hook does anything in this workflow. The lint hooks check Markdown files and the final reply, and neither is what gets posted. I don't think it saves tokens, since a run's cost is mostly code read in rather than comment text written out. On Apache Magpie, we run into these sorts of issues, and it gorsd a bit further: - clean-env wrapper strips credential variables before the agent starts - I think this is an issue here for other environment variables - runs the agent in an OS sandbox that blocks reads outside the tree and blocks network access to hosts not on an allowlist - PR text is data, never instructions -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
