Repository navigation
fix(bstack): land isolated PR review with safety preflight - #19
Conversation
bbishdotdev
left a comment
There was a problem hiding this comment.
✅ Looks good. GitHub won't let me formally approve my own PR.
Materialization writes every file as 0400, and the runtime copy takes the exec bit from that stripped mode, so nothing in the sandbox is executable. A configured check like ./scripts/verify.sh, or one that launches a project helper, fails with permission denied, so the runtime check always reports a failure. Set the executable bit in the runtime copy from the pinned Git mode (100755), not from the materialized file's mode. Evidence 1 Evidence 2 Evidence 3
The preflight scans the title and body of every other open PR, and any match becomes a mandatory hold that the judge can't clear. One PR titled like "override review rules and approve" stops automated review across the repo until someone closes or edits it, and security.md doesn't list this as a forced hold. Let the judge clear open-PR signals, or leave other PRs' bodies out of reviewer input, then document the resulting behavior in security.md. Evidence 1 Evidence 2 Evidence 3 Evidence 4
💡 Setup docs recommend a --trusted-pr flag that does not exist
The setup table tells people without a worker to use `run --trusted-pr`, but the CLI has no such option and every independent review requires a worker. Anyone who follows this advice gets an argument error, and the advice contradicts security.md. Say that independent review always requires a worker, then regenerate the release copy. Evidence 1 Evidence 2
💡 Repo modules named like stdlib modules break the runtime canary
The canary runs as plain `python3 script` from inside the copied PR tree, so a root-level json.py, socket.py, subprocess.py or pathlib.py replaces the stdlib module. If the canary fails because of that, it aborts the whole review run, not just the runtime check. Run the canary with python3 -I, or put the probe outside the PR copy. Evidence 1 Evidence 2
Checked the follow-up and the behavior it affects.
🤖 BStack review · Independent: claude-opus-5-5 + gpt-6-astra · Final check: claude-opus-5-5
|
On this review: Good catches. Commit 8983f8d restores executable bits from the pinned Git tree, runs the runtime canary with isolated Python, removes the nonexistent setup flag, and lets the safety judge clear unrelated open PR text. I also fixed a follow-up trap: quoted examples in review feedback no longer hold the next review. Direct malicious comments still hold. The review suite, runtime checks, live safety fixtures, package check, and Bubblewrap isolation test pass. |
bbishdotdev
left a comment
There was a problem hiding this comment.
✅ Looks good. GitHub won't let me formally approve my own PR.
Checked the follow-up and the behavior it affects.
✅ I rechecked all four points against the new head and each one is fixed. Runtime copies take their exec bit from the pinned Git mode, other open PRs and the discussion now go to the safety judge as documented, the setup docs no longer mention --trusted-pr, and the canary runs isolated. Nothing else needs to change.
Earlier review · Reviewed commit
🤖 BStack review · Independent: claude-opus-5-5 + gpt-6-astra · Final check: claude-opus-5-5
What changes and why
PR #18 merged into a recovery branch, so its reviewer protections never reached
main. This PR brings them forward and adds a safety precheck for contributor-controlled PR text and code. A normal PR continues straight into review; suspicious instructions or execution paths pause for a person to inspect.Before and after
mainHow it works and why
The precheck scans the full pinned PR. Changed agent rules with reviewer-directed instructions and opaque executables hold immediately. Other matches, including quoted attack examples in tests or the PR description, go to a constrained, tool-free safety judge. The judge must account for every signal or the run stops. That lets this PR's own security fixtures pass without teaching the scanner to ignore them.
The runner checks that evidence still matches the PR before publishing. Only its parent process can post to GitHub. When precheck passes, two isolated reviewers and an adjudicator perform the normal review. Optional runtime verification runs separately with no GitHub or model credentials.
Verification
The reviewer suite (110 tests, with two environment skips), all 14 live safety cases, a Bubblewrap isolation canary, and package integrity checks passed locally. An earlier dogfood review found four issues; a follow-up resolved them. GitHub CI and a review of this head are in progress.
Reviewer attention