fix: close security review blockers - #1
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
kaipingyang
left a comment
There was a problem hiding this comment.
Thanks for the broad security-hardening work. I found several merge blockers that need to be resolved before this can land:
- Prompt/tool contract regression: explicit
codeagent_client(chat=...)disables process delegation, but the system prompt andtest-prompt-tool-alignment.Rstill requireTeamRun. Reproduced on this head:missing = "TeamRun". - Plan-mode escape:
.gate_decide()unconditionally allowsExitPlanModebefore metadata, sets, overrides, and deny rules. A session started in plan mode can escape the read-only boundary. - Deny precedence mismatch: per-tool/capability
allowreturns before explicit deny rules, contrary to the documented "deny always wins" contract. - Bash environment scrubbing is ineffective: base
system2(env=...)augments/overrides variables rather than replacing the inherited environment, so unlisted secrets remain visible to child processes. - Policy sandbox gaps: path extraction does not constrain paths embedded in Bash/RunR code or
Glob(pattern=...), and exec-capability tools can bypassnetwork="deny". - ExploreData risk contract: it executes arbitrary
eval(parse())code while public docs/tool annotations still claim sandboxed/read-only/non-destructive behavior.
Please add regression coverage for each boundary. Before merge, run the complete testthat suite, R CMD check, install the built package and run installed-package smoke tests, and verify both classic and page_chat layouts in real Chromium. The PR should remain draft until those gates pass.
All requested security changes were implemented in 01277b6 and passed independent security review, full R package validation, installed smoke, pkgdown, and real Chromium E2E.
kaipingyang
left a comment
There was a problem hiding this comment.
Security blockers cleared by independent review. Full testthat, R CMD check, exact-tarball installed smoke, pkgdown, and real Chromium classic/page_chat validation passed.
Summary
Closes all security and integration blockers raised in the owner review and subsequent independent reviews.
team_lead()to the saferdont_askdefault and updates documentationIndependent gates
SECURITY_BLOCKERS_CLEAREDREADY_FOR_FULL_REGRESSIONVerification
devtools::test(reporter = "summary"): 3342 assertions, 0 failures; 13 known locale warnings, 4 optional skipsdevtools::check(error_on = "warning", cran = FALSE, document = FALSE): 0 errors, 0 warnings, 4 non-blocking notes; check tests OK45f9e000fb80d91a8a47ea3a760580e2757beee162a0b3f2d7c909ec171f856dpkgdown::check_pkgdown(): No problems foundcore,pass,redact,blockPASScoreand width assertion PASSgit diff --check: PASScodegraph sync: already up to dateValidated commit:
01277b62fa114da3cace15620c1e2241c31e685f.