fix: close security review blockers

.Rbuildignore

@@ -23,4 +23,5 @@
 ^lessons$
 ^to_kiro$
 ^otelsdk-install-issue\.md$
+^SECURITY-FIXES\.md$
 ^inst/experiments$

DESCRIPTION

@@ -31,11 +31,13 @@ Imports:
     keypress,
     base64enc,
     commonmark,
+    xml2,
     bsicons,
     reactable,
     later,
     rlang,
-    watcher
+    watcher,
+    processx
 Suggests:
     codetools,
     clipr,
@@ -88,4 +90,4 @@ Remotes:
 URL: https://kaipingyang.github.io/codeagent, https://github.com/kaipingyang/codeagent
 BugReports: https://github.com/kaipingyang/codeagent/issues
 Config/roxygen2/version: 8.0.0
-RoxygenNote: 7.3.2
+RoxygenNote: 7.3.3

NEWS.md

@@ -1,7 +1,19 @@
 # codeagent 0.2.3
 
-This backward-compatible patch release adds optional Liquid Glass theming and
-fixes default tool-card expansion without changing public APIs.
+This patch release adds optional Liquid Glass theming, fixes tool-card behavior,
+and closes security/integration blockers found during PR review. It includes one
+intentional safe-default change: `team_lead()` now defaults to `"dont_ask"`.
+
+## Security and reliability
+
+* Made matching explicit deny rules absolute across capability allows, per-tool overrides, modes, and read fast paths; initial plan sessions can no longer invoke `ExitPlanMode` without a trusted in-session `EnterPlanMode` transition.
+* Replaced sandboxed Bash `system2()` execution with `processx` argv execution and a full replacement environment, confined Glob patterns and symlink results, and made the Data Shield portable policy block every non-delegated exec tool—including path-declared `Lint`, whose project `.lintr` may execute R code—until a real OS sandbox is available.
+* Made PreToolUse installation and runtime exceptions fail closed, and rebuilt delegation guidance from the final live tool registry so missing/failed Agent or TeamRun registration is never advertised.
+* Preserved dynamic plan-mode state when a Shiny permission/tool refresh rolls back.
+
+## Behavior changes
+
+* Changed `team_lead()`'s default `permission_mode` from `"bypass"` to `"dont_ask"`. Existing trusted write workflows must now opt in explicitly to a more permissive mode; read-only review workflows continue to work with the safer default.
 
 ## Shiny UI
 
@@ -273,13 +285,14 @@ upstream integrations, Shiny presentation, safety, and documentation.
   payloads/audit contain metadata only, never the raw result.
 
 * **Portable sandbox policy**: new `shield_sandbox()` keeps project/session-temp
-  `rwx` and process execution by default while the central gate validates all
-  explicit path arguments against project/protected/temp roots, follows real
-  paths to reject symlink escape, enforces per-root `r/rw/rwx`, and can deny
-  network/exec capabilities. `backend="auto"` honestly falls back to policy (or
-  blocks in required mode) because a full OS process adapter is not yet wired;
-  coverage/audit report the fallback. btw file tools are also covered (their cwd
-  guard permits symlink escape), and btw RunR is not treated as an OS sandbox.
+  path capabilities at `rwx` while the central gate validates explicit path
+  arguments against project/protected/temp roots, follows real paths to reject
+  symlink escape, and enforces per-root `r/rw/rwx`. Because portable path policy
+  is not process isolation, every non-delegated exec tool fails closed until a
+  full OS adapter is available; this includes path-declared tools that can run
+  project configuration. `backend="auto"` honestly falls back to policy (or
+  blocks in required mode); coverage/audit report the fallback. btw file tools
+  are also covered, and btw RunR is not treated as an OS sandbox.
 
 * **Per-tool/agent Shield policy**: new `shield_tool_policy()` supports exact or
   `*`-glob rules with `scan` (default), explicit audited `bypass`, and `deny`

R/addin.R

@@ -74,7 +74,6 @@ codeagent_addin_selection <- function() {
 }
 
 # Insert text at the cursor position in the active source editor.
-# Used by future addin features (e.g. /inline-edit that patches the file).
 .insert_at_cursor <- function(text) {
   if (!requireNamespace("rstudioapi", quietly = TRUE)) return(invisible(NULL))
   if (!rstudioapi::hasFun("insertText")) return(invisible(NULL))

R/async_agent.R

@@ -24,7 +24,10 @@ NULL
 
 .bg_ensure_daemons <- function(n = 2L) {
   if (isTRUE(.bg_state$daemons)) return(invisible())
-  ok <- tryCatch({ mirai::daemons(n, .compute = .BG_COMPUTE); TRUE },
+  ok <- tryCatch({
+    .start_secure_mirai_daemons(n, .compute = .BG_COMPUTE)
+    TRUE
+  },
                  error = function(e) FALSE)
   if (isTRUE(ok)) .bg_state$daemons <- TRUE
   invisible()
@@ -40,28 +43,50 @@ NULL
 
 #' Spawn a background sub-agent. Returns the task id immediately (fire-and-forget).
 #' @keywords internal
-.bg_spawn <- function(prompt, model = NULL, cwd = getwd()) {
+.bg_spawn <- function(prompt, model = NULL, cwd = getwd(),
+                      security_context = NULL) {
+  context_supplied <- !is.null(security_context)
+  if (context_supplied && is.null(security_context$backend))
+    return(structure(
+      "[background agent unavailable: parent Chat cannot be safely reconstructed]",
+      class = "bg_error"))
   if (!.bg_available())
     return(structure("[background agents require the mirai package]",
                      class = "bg_error"))
   .bg_ensure_daemons()
-  model    <- model %||% Sys.getenv("CODEAGENT_MODEL", "")
-  base_url <- Sys.getenv("CODEAGENT_BASE_URL", "")
-  api_key  <- Sys.getenv("CODEAGENT_API_KEY", "")
+  security_context <- security_context %||%
+    .worker_security_context(permission_mode = "dont_ask", cwd = cwd)
+  backend <- security_context$backend %||% NULL
+  model <- backend$model %||% model %||% Sys.getenv("CODEAGENT_MODEL", "")
+  base_url <- if (is.null(backend)) Sys.getenv("CODEAGENT_BASE_URL", "") else ""
+  api_key <- if (is.null(backend)) Sys.getenv("CODEAGENT_API_KEY", "") else ""
+  legacy_env <- is.null(backend)
+  security_json <- tryCatch(
+    .worker_security_context_json(security_context),
+    error = function(e) NULL
+  )
+  if (is.null(security_json))
+    return(structure("[invalid background-agent security context]",
+                     class = "bg_error"))
   m <- tryCatch(
     mirai::mirai(
       {
-        Sys.setenv(CODEAGENT_BASE_URL = base_url, CODEAGENT_API_KEY = api_key,
-                   CODEAGENT_MODEL = model)
+        if (isTRUE(legacy_env))
+          Sys.setenv(CODEAGENT_BASE_URL = base_url, CODEAGENT_API_KEY = api_key,
+                     CODEAGENT_MODEL = model)
+        else
+          Sys.setenv(CODEAGENT_MODEL = model)
         suppressMessages(suppressWarnings(library(codeagent)))
         tryCatch({
-          client <- codeagent::codeagent_client(permission_mode = "bypass",
-                                                 cwd = cwd, btw_groups = NULL)
+          client <- codeagent:::.worker_client_from_json(
+            model, security_json)
           codeagent::codeagent(client, prompt)
         }, error = function(e) paste0("[Error] ", conditionMessage(e)))
       },
       prompt = prompt, model = model, base_url = base_url,
-      api_key = api_key, cwd = cwd, .compute = .BG_COMPUTE),
+      api_key = api_key, cwd = cwd, security_json = security_json,
+      legacy_env = legacy_env,
+      .compute = .BG_COMPUTE),
     error = function(e) NULL)
   if (is.null(m))
     return(structure("[failed to spawn background agent]", class = "bg_error"))
@@ -209,15 +234,16 @@ NULL
 }
 
 # Spawn from a user slash command; returns a feedback string (never errors).
-.bg_slash_spawn <- function(task, data_shield = NULL) {
+.bg_slash_spawn <- function(task, data_shield = NULL,
+                            security_context = NULL) {
   task <- trimws(task %||% "")
   if (!nzchar(task)) return("Usage: /bg <task>")
   if (inherits(data_shield, "DataShield"))
     return(paste0(
       "Background agents are disabled while Data Shield is active: ",
       "the mirai worker cannot safely inherit this session's protected-data index. ",
       "Use the foreground Agent tool instead."))
-  id <- .bg_spawn(task)
+  id <- .bg_spawn(task, security_context = security_context)
   if (inherits(id, "bg_error")) return(paste0("Background agents unavailable: ", unclass(id)))
   sprintf("Started background sub-agent #%s. Its result will appear on a later turn.", id)
 }

R/chat_commands.R

@@ -29,7 +29,8 @@ NULL
 .chat_command_result <- function(name, args = "",
                                   n_tokens = 0L, model_limit = 200000L,
                                   n_turns = 0L, sessions = list(),
-                                  data_shield = NULL) {
+                                  data_shield = NULL,
+                                  security_context = NULL) {
   name <- name %||% ""
   args <- args %||% ""
 
@@ -65,7 +66,8 @@ NULL
     sessions = list(action = "append",
                     feedback = .format_sessions_feedback(sessions)),
 
-    bg = list(action = "append", feedback = .bg_slash_spawn(args, data_shield)),
+    bg = list(action = "append", feedback = .bg_slash_spawn(
+      args, data_shield, security_context)),
 
     bgstatus = list(action = "append", feedback = .bg_status_text()),
 

R/code_audit.R

@@ -154,6 +154,37 @@ NULL
 # text-audited") rather than read.
 .AUDIT_READ_EXTS <- c("R", "r", "Rmd", "rmd", "qmd", "cpp", "c", "h", "hpp")
 
+# POSIX `test -f` reliably distinguishes regular files from FIFOs on platforms
+# where base R's file_test("-f") may report a FIFO as regular. Use argv execution
+# and fail closed if the type cannot be verified.
+.audit_is_regular_file <- function(path) {
+  if (.Platform$OS.type == "windows")
+    return(isTRUE(utils::file_test("-f", path)))
+  test_bin <- unname(Sys.which("test"))
+  if (!nzchar(test_bin)) return(FALSE)
+  status <- tryCatch(
+    processx::run(test_bin, c("-f", path), stdout = "|", stderr = "|",
+                  error_on_status = FALSE, timeout = 2)$status,
+    error = function(e) 1L)
+  identical(as.integer(status), 0L)
+}
+
+
+.audit_read_bounded <- function(path, max_bytes, timeout_ms = 5000) {
+  rscript <- file.path(
+    R.home("bin"), if (.Platform$OS.type == "windows") "Rscript.exe" else "Rscript")
+  code <- paste0(
+    "a <- commandArgs(TRUE); ",
+    "con <- file(a[[1L]], 'rb'); on.exit(close(con), add=TRUE); ",
+    "cat(readChar(con, nchars=as.integer(a[[2L]]), useBytes=TRUE))")
+  out <- processx::run(
+    rscript, c("--vanilla", "-e", code, path, as.character(as.integer(max_bytes))),
+    stdout = "|", stderr = "|", timeout = as.numeric(timeout_ms) / 1000,
+    error_on_status = FALSE, cleanup_tree = TRUE)
+  if (!identical(as.integer(out$status), 0L))
+    stop("isolated source read failed", call. = FALSE)
+  out$stdout %||% ""
+}
 # Decide whether a referenced path is safe to read: it must resolve to a real
 # location UNDER project_root (symlink-escape resolved by
 # .data_shield_resolve_path -> normalizePath) AND carry a source-file
@@ -176,18 +207,9 @@ NULL
                 reason = sprintf("non-source extension '%s'", ext)))
   if (!file.exists(resolved))
     return(list(ok = FALSE, resolved = resolved, reason = "file does not exist"))
-  # Must be a REGULAR file, not a directory/FIFO/socket/device (kiro round-3 #dir
-  # + round-4 #8). A directory read()s to nothing (silently -> risk=none); a FIFO
-  # BLOCKS `file(open="rb")` forever waiting for a writer (DoS). file.info()$isdir
-  # only distinguishes directories, so a FIFO/socket slips through. Use POSIX
-  # `test -f` (true ONLY for a regular file) to reject every non-regular type.
   if (dir.exists(resolved))
     return(list(ok = FALSE, resolved = resolved, reason = "is a directory"))
-  is_regular <- tryCatch(
-    identical(0L, suppressWarnings(system2("test", c("-f", shQuote(resolved)),
-                                           stdout = FALSE, stderr = FALSE))),
-    error = function(e) FALSE)
-  if (!isTRUE(is_regular))
+  if (!.audit_is_regular_file(resolved))
     return(list(ok = FALSE, resolved = resolved,
                 reason = "not a regular file (FIFO/socket/device rejected)"))
   list(ok = TRUE, resolved = resolved, reason = NA_character_)
@@ -241,16 +263,29 @@ NULL
     if (inherits(shield, "DataShield")) {
       read_failed <- FALSE
       content <- tryCatch({
-        con <- file(dec$resolved, "rb"); on.exit(close(con), add = TRUE)  # rb: bounded binary read
-        # TOCTOU guard (kiro round-2 #13 + round-3): re-resolve the opened path
-        # and confirm it still matches the vetted target under project_root.
+        # TOCTOU guard before launching the bounded reader. A later regular-file
+        # to FIFO/device swap can still race this check, but it can block only
+        # the isolated child, which processx terminates at the hard timeout.
         recheck <- tryCatch(normalizePath(dec$resolved, winslash = "/", mustWork = TRUE),
                             error = function(e) NA_character_)
         if (is.na(recheck) || !identical(recheck, dec$resolved) ||
-            !.data_shield_path_under(recheck, normalizePath(project_root, winslash = "/", mustWork = FALSE)))
+            !.data_shield_path_under(
+              recheck, normalizePath(project_root, winslash = "/", mustWork = FALSE)))
           stop("path changed after validation (TOCTOU)")
-        readChar(con, nchars = max_bytes, useBytes = TRUE)
+        .audit_read_bounded(dec$resolved, max_bytes)
       }, error = function(e) { read_failed <<- TRUE; NULL })
+      # If readChar succeeded, ensure the content is valid UTF-8 by
+      # re-encoding it (this also strips any trailing multi-byte char that
+      # was truncated in the middle).
+      if (!isTRUE(read_failed) && is.character(content) && length(content) == 1L) {
+        content <- enc2utf8(content)
+        # If the last character is incomplete (common with useBytes=TRUE),
+        # nchar(..., type="chars") will still work but the character may
+        # be invalid. Replace any invalid UTF-8 sequences.
+        content <- tryCatch(
+          intToUtf8(utf8ToInt(content), multiple = FALSE),
+          error = function(e) iconv(content, from = "UTF-8", to = "UTF-8", sub = ""))
+      }
       # A read/TOCTOU failure must NOT be silently dropped to risk=none (kiro
       # round-3): record it as blocked so the overall risk escalates to block.
       if (isTRUE(read_failed)) {

R/compaction.R

@@ -1525,7 +1525,12 @@ CompactionController <- R6::R6Class(
         }
       )
       if (!identical(decision$reason, "callback_error")) {
-        if (!isTRUE(decision$success) && decision$summary_calls > 0L) {
+        # Increment failure counter only for genuine failures — NOT when
+        # post_compact_still_large (the pipeline ran but couldn't compact
+        # enough below threshold) or over_threshold_full_disabled.
+        if (!isTRUE(decision$success) && decision$summary_calls > 0L &&
+            !identical(decision$reason, "no_safe_group") &&
+            !grepl("still_large|full_disabled", decision$reason %||% "")) {
           private$failures <- private$failures + 1L
         } else if (isTRUE(decision$success)) {
           private$failures <- 0L

R/context.R

@@ -113,8 +113,8 @@ NULL
     v <- suppressWarnings(as.integer(env))
     if (!is.na(v) && v > 0L) return(v)
   }
-  # 2. [1m] suffix (= has1mContext, context.ts:35)
-  if (!is.null(model) && grepl("\\[1m\\]", model, ignore.case = TRUE)) return(1000000L)
+  # 2. [1m] suffix (= has1mContext, context.ts:35). Match only at end of model name.
+  if (!is.null(model) && grepl("\\[1m\\]$", model, ignore.case = TRUE)) return(1000000L)
   # 3. Capability (provider value or table); only trust >= 100K (= CC guard)
   cap <- .model_capability_tokens(model, chat)
   if (!is.na(cap) && cap >= 100000L) return(cap)
@@ -191,7 +191,7 @@ calculate_token_warning_state <- function(token_usage, model, chat = NULL) {
     percent_left  = max(0L, as.integer(round((threshold - token_usage) / threshold * 100))),
     above_warning = token_usage >= threshold - .WARNING_THRESHOLD_BUFFER,
     above_error   = token_usage >= threshold - .ERROR_THRESHOLD_BUFFER,
-    above_compact = enabled && token_usage >= .auto_compact_threshold(model, chat),
+    above_compact = enabled && token_usage >= threshold,
     at_blocking   = token_usage >= eff - .MANUAL_COMPACT_BUFFER
   )
 }