Skip to content

fix: close security review blockers - #1

Merged
kaipingyang merged 3 commits into
mainfrom
process
Sep 14, 2026
Merged

kaipingyang merged 3 commits into
mainfrom
process

Conversation

@dangerously123

@dangerously123 dangerously123 commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

Summary

Closes all security and integration blockers raised in the owner review and subsequent independent reviews.

  • makes explicit deny rules absolute and closes plan-mode escape paths
  • replaces inherited Bash environments and corrects processx timeout units
  • fail-closes path/glob containment, portable Data Shield exec policy, PreToolUse hooks, and child Agent shielding
  • keeps live tool registries, delegation prompts, WEAR tools, permission state, and model switching transactional
  • hardens AuditCode against special-file/TOCTOU blocking
  • changes team_lead() to the safer dont_ask default and updates documentation

Independent gates

  • SECURITY_BLOCKERS_CLEARED
  • READY_FOR_FULL_REGRESSION

Verification

  • devtools::test(reporter = "summary"): 3342 assertions, 0 failures; 13 known locale warnings, 4 optional skips
  • devtools::check(error_on = "warning", cran = FALSE, document = FALSE): 0 errors, 0 warnings, 4 non-blocking notes; check tests OK
  • clean source tarball SHA-256: 45f9e000fb80d91a8a47ea3a760580e2757beee162a0b3f2d7c909ec171f856d
  • exact tarball install plus installed-package smoke: PASS
  • pkgdown::check_pkgdown(): No problems found
  • real Chromium, installed package:
    • classic: core, pass, redact, block PASS
    • page_chat: core and width assertion PASS
  • staged high-confidence secret/endpoint scan: 0 findings
  • git diff --check: PASS
  • codegraph sync: already up to date

Validated commit: 01277b62fa114da3cace15620c1e2241c31e685f.

tong3-wang_bgne and others added 2 commits September 11, 2026 15:12
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

@kaipingyang kaipingyang left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks for the broad security-hardening work. I found several merge blockers that need to be resolved before this can land:

  1. Prompt/tool contract regression: explicit codeagent_client(chat=...) disables process delegation, but the system prompt and test-prompt-tool-alignment.R still require TeamRun. Reproduced on this head: missing = "TeamRun".
  2. Plan-mode escape: .gate_decide() unconditionally allows ExitPlanMode before metadata, sets, overrides, and deny rules. A session started in plan mode can escape the read-only boundary.
  3. Deny precedence mismatch: per-tool/capability allow returns before explicit deny rules, contrary to the documented "deny always wins" contract.
  4. 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.
  5. Policy sandbox gaps: path extraction does not constrain paths embedded in Bash/RunR code or Glob(pattern=...), and exec-capability tools can bypass network="deny".
  6. 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.

@kaipingyang kaipingyang changed the title Process fix: close security review blockers Sep 14, 2026
@kaipingyang
kaipingyang dismissed their stale review September 14, 2026 04:42

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 kaipingyang left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Security blockers cleared by independent review. Full testthat, R CMD check, exact-tarball installed smoke, pkgdown, and real Chromium classic/page_chat validation passed.

@kaipingyang
kaipingyang marked this pull request as ready for review September 14, 2026 04:42
@kaipingyang
kaipingyang merged commit 4dc479d into main Sep 14, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants