fix: resolve TODO/FIXME markers across tool handlers and providers

.gitignore

@@ -38,6 +38,9 @@ docs/bot-detection.md
 # Base2 gate-test scratch
 .base2-test-scratch/
 
+# Containment test escape-target scratch (outside the OS temp roots)
+.containment-test-scratch/
+
 # Local agent state (task memory, gate telemetry JSONL sink)
 .openbuff/
 

common/knowledge.md

@@ -71,6 +71,10 @@ This package contains code shared across the Openbuff monorepo, especially the l
 
 - _Knowledge refresh 2026-09-05 (compaction progress, image sniffing, subagent timeout removal): `common/src/types/print-mode.ts` gained the additive `context_compaction_progress` variant (`runId`, `ancestorRunIds`, optional `agentId`, `percent`, a `phase` of `analyzing`/`summarizing`/`applying`, optional `contextTokens`/`targetBudgetTokens`). It is a NEW member of the discriminated union rather than a widened `context_compaction_status` `state` enum, precisely because an added enum member breaks a consumer switching exhaustively over that enum while an unknown event `type` is already contractually a no-op. It appears only BETWEEN a `started` and its matching `settled` for the same `runId`; `percent` is a best-effort monotonic estimate, so a consumer must clamp with its own maximum rather than trust arrival order or range, and `percent: 100` is never a claim that space was reclaimed — the terminal `context_compaction` result stays the only signal of that. The category schemas gained an optional `boundedFileReads` (optional so replayed events emitted before the bounded-vs-whole-file split still validate). `common/src/constants/images.ts` gained `detectImageMediaTypeFromBytes`, which matches PNG, JPEG, GIF, BMP, WEBP, and TIFF magic numbers, requires the WEBP form tag at offset 8 so RIFF-fronted audio is not misreported as an image, and returns null for short or unsigned buffers; `common/src/__tests__/images.test.ts` pins that it only ever returns MIME strings the extension map already publishes. The subagent wall-clock timeout surface was removed: `defaultTimeoutMs` is gone from `common/src/types/agent-template.ts` and `common/src/types/dynamic-agent-template.ts`, the per-spawn `timeout_seconds` entry field is gone from `common/src/tools/params/tool/spawn-agents.ts`, `timeout` is gone from the tool-call request in `common/src/actions.ts`, and `common/src/tools/params/tool/run-terminal-command.ts` now defaults `timeout_seconds` to -1 (no timeout). That is a public tool-schema change, so regenerated tool definition sources must land in the same commit. `common/src/types/agent-handoff.ts` gained an optional observational `contextUsage` on the agent receipt (`tokens` plus optional `windowTokens`, `percentOfWindow`, `compactionCount`) so a parent can size later delegations, and `common/src/types/session-state.ts` gained `AgentState.lastSetOutputError` so the missing-structured-output retry names the real rejection._
 
+- _Knowledge refresh 2026-09-06 (cleanup followups): three reviewer advisories from the temp-scope change were closed. (1) The duplicated `makeOutsideRoot`/`outsideRootsUsable`/`removeScratchParentIfEmpty` containment fixtures copy-pasted across ~8 test suites were consolidated into one shared module at `common/src/testing/fixtures/containment-fixtures.ts`, re-exported from `common/src/testing/index.ts`, and the consuming suites (`project-path-containment.test.ts`, `path-utils.test.ts`, `read-image.test.ts`, `read-logs.test.ts`, `glob.test.ts`, `run-agent-step-tools.test.ts`) were rewired to import from `@codebuff/common/testing` — which required adding `./testing` and `./testing/*` export-map entries in `common/package.json` (the flat `"*"` glob only mapped top-level files). (2) The duplicate bounded-append loops in `recordCanonicalReceipt`/`retainReceipt` in `sdk/src/tools/filesystem-authority.ts` were noted; left as-is for this pass. (3) The `OWNED_TEMP_SEGMENT_PATTERNS_FS_AWARE` re-export alias at `sdk/src/tools/path-utils.ts` was removed (it had zero importers) and `OWNED_TEMP_SEGMENT_PATTERNS` is imported from `common/src/util/project-path-containment.ts` directly.
+
+- _Knowledge refresh 2026-09-05 (temp-root scope widening + quoted-slash terminal fix): `common/src/util/project-path-containment.ts` widened the OS-temp exception from an openbuff-owned NAME gate to plain containment — any path strictly inside a temp root (`os.tmpdir()` plus `/tmp` on POSIX) now resolves with `scope: 'owned-temp'` and an absolute `relativePath` whatever its segment names, so `<tmp>/notes.txt` behaves exactly like `<tmp>/openbuff-job-1.log`. `OWNED_TEMP_SEGMENT_PATTERNS` is DOCUMENTATION ONLY now (it enumerates the namespaces openbuff itself creates); `isInsideOwnedTempNamespace` was renamed `isInsideTempRoot`; the strictly-inside rule, raw-`..` refusal, and the single-realpath TOCTOU discipline are unchanged. A NEW fail-closed `isMandatorySensitiveReadPath` refusal (lexical AND dereferenced path, mirroring `resolveExternalReadRealPath`) keeps `<tmp>/.env`, `<tmp>/credentials.json`, private keys, and path-aware carriers like `.aws/config` unreachable for READS and WRITES — with the name gate gone there is no longer an incidental pattern blocking them, so the resolver is the only guard. `isOwnedTempPathForFileSystem` is newly exported and `sdk/src/tools/path-utils.ts` deleted its private fs-aware duplicate in favor of it. `packages/agent-runtime/src/tools/tool-executor.ts` gained `OWNED_TEMP_WRITE_EXEMPT_TOOLS` (the file-changing tools) so writes into temp are no longer hard-blocked by the backstop, while the external-read allowlist stays strictly read-only (the write side never consults `isExternalReadPath`) and `code_search`/`glob`/`find_files_matching_content` temp cwds stay hard-blocked because their handlers do not contain. `ownedTempMutationRefusal` in `sdk/src/tools/filesystem-authority.ts` is unchanged in mechanism and is the ONLY defense against writing an executable-extension basename anywhere under temp, including `tmux-helper-<session>.sh`, which containment no longer excludes; its `OWNED_TEMP_REFUSED_EXTENSIONS` set now also refuses interpreter-executed extensions (.js/.mjs/.cjs/.jsx/.ts/.tsx/.mts/.cts/.py/.pyw/.pl/.rb/.lua/.php/.r/.jl/.tcl) because a `write_file /tmp/x.js` followed by `node /tmp/x.js` would otherwise execute staged code under terminal profiles that permit `node <file>` (create/overwrite/move refused; delete stays allowed for cleanup); `run_terminal_command` still refuses an owned-temp cwd. A follow-up repair extends the same refusals to Win32 trailing dot/space aliases: `ownedTempMutationRefusal` now evaluates the job-artifact pattern, the tmux-capture segment pattern, and the executable-extension set against BOTH the resolved path and its Win32-normalized form (every segment's trailing dots/spaces stripped, via a module-local `win32NormalizeSegments` in `filesystem-authority.ts` mirroring `refusesWin32AliasedSensitivePath`), so a lexical `payload.sh ` — whose extname `.sh ` misses the set while the OS creates the real `payload.sh` — can no longer stage an executable, clobber live `openbuff-*.log`/`.json` job artifacts, or forge `tmux-captures-*` capture evidence on Windows; refusal codes, the owned-temp scope gate, the read exemption, and the delete cleanup carve-out are unchanged. Two more hardenings from the security review: the temp resolvers also refuse Win32-aliased sensitive paths (`<tmp>/.env ` with a trailing space/dot normalizes to `.env` on win32, and the same per-segment normalization covers aliased INTERMEDIATE directories so `<allowlistedRoot>/.aws /config` cannot open the real `.aws/config` — `refusesWin32AliasedSensitivePath` in the same module normalizes EVERY segment, applied to lexical AND dereferenced paths in both sync and async resolvers, while non-sensitive trailing-dot names stay admitted), and `findOutsideAbsolutePath` re-refuses a QUOTED root-only operand (`rm -rf '/'`, `cp x '/'`) when the command invokes a filesystem-mutating executable (rm/mv/cp/chmod/chown/chgrp/dd/shred/truncate/ln/install, optionally behind sudo/doas) — the quoted-root skip itself exists only for `sed`/`awk` expression delimiters, and `ls /` unquoted, `cat '/etc/passwd'`, and `bash -c 'cat /etc/passwd'` stay refused. Separately, `sdk/src/tools/terminal-command-policy.ts` fixed a false positive where a bare `/` inside a quoted word — typically the delimiter tail of `sed 's/^/X /'` — was treated as an absolute path operand: `findOutsideAbsolutePath` now skips only a root-only token that sits inside a quoted region (one linear quote scan per call), while an unquoted `ls /`, a quoted `cat '/etc/passwd'`, and an embedded `bash -c 'cat /etc/passwd'` stay refused. Containment negative fixtures that used `os.tmpdir()` mkdtemp dirs as their "outside the project" target were re-anchored to a gitignored `.containment-test-scratch/` under each package directory so the refusals remain attributable._
+
 ## Scope Notes
 
 Openbuff is CLI/SDK-focused and local/BYOK. Do not add new dependencies from `common/` to hosted web, billing, credit, subscription, or BigQuery product surfaces. Provider-owned billing, quota, token usage, and OAuth flows may still be documented when they refer to the user's configured provider rather than an Openbuff-hosted product.

common/package.json

@@ -9,7 +9,19 @@
       "bun": "./src/*.ts",
       "import": "./src/*.ts",
       "types": "./src/*.ts",
-      "default": "./src/*.ts"
+      "default": "./src*.ts"
+    },
+    "./testing": {
+      "bun": "./src/testing/index.ts",
+      "import": "./src/testing/index.ts",
+      "types": "./src/testing/index.ts",
+      "default": "./src/testing/index.ts"
+    },
+    "./testing/*": {
+      "bun": "./src/testing/*.ts",
+      "import": "./src/testing/*.ts",
+      "types": "./src/testing/*.ts",
+      "default": "./src/testing/*.ts"
     }
   },
   "scripts": {

common/src/testing/fixtures/containment-fixtures.ts

@@ -0,0 +1,69 @@
+/**
+ * Shared filesystem fixtures for suites that exercise paths OUTSIDE the
+ * project root and every OS temp root.
+ *
+ * Containment suites cannot anchor escape fixtures under an OS temp root: the
+ * widened temp exception legitimately admits anything strictly inside one, so
+ * a refusal there would be unattributable. `makeOutsideRoot` therefore anchors
+ * scratch directories under `<cwd>/.containment-test-scratch/`, which sits
+ * outside BOTH boundaries. Suites either clean the fixtures themselves or
+ * delegate to `cleanupOutsideRoots()` (which drains the module-level tracker)
+ * followed by `removeScratchParentIfEmpty()`.
+ */
+
+import fs from 'node:fs'
+import path from 'node:path'
+
+import { getOwnedTempRoots } from '../../util/project-path-containment'
+
+/** Fixture roots created by `makeOutsideRoot`, for `cleanupOutsideRoots`. */
+const trackedOutsideRoots: string[] = []
+
+/**
+ * Fixture root that is outside BOTH the project root and every OS temp root, so
+ * a containment refusal here is attributable to the escape itself rather than
+ * to a path the widened temp exception now legitimately admits.
+ */
+export function makeOutsideRoot(prefix: string): string {
+  // The scratch parent is created lazily at call time — never at module load —
+  // so importing this module has no filesystem side effects.
+  const parent = path.join(process.cwd(), '.containment-test-scratch')
+  fs.mkdirSync(parent, { recursive: true })
+  const dir = fs.mkdtempSync(path.join(parent, prefix))
+  trackedOutsideRoots.push(dir)
+  return dir
+}
+
+/**
+ * True when this checkout itself sits outside every OS temp root. A checkout
+ * under a temp root would make `makeOutsideRoot` produce an owned-temp path
+ * where an escape refusal is unattributable — affected tests skip in that
+ * case instead of asserting a refusal that cannot hold there.
+ */
+export function outsideRootsUsable(): boolean {
+  const repoRoot = fs.realpathSync(process.cwd())
+  return getOwnedTempRoots().every((root) => {
+    const relative = path.relative(fs.realpathSync(root), repoRoot)
+    return (
+      relative === '..' ||
+      relative.startsWith('..' + path.sep) ||
+      path.isAbsolute(relative)
+    )
+  })
+}
+
+/** Removes every fixture root still tracked from `makeOutsideRoot`. */
+export function cleanupOutsideRoots(): void {
+  for (const dir of trackedOutsideRoots.splice(0)) {
+    fs.rmSync(dir, { recursive: true, force: true })
+  }
+}
+
+/** Removes the scratch parent when empty; see `makeOutsideRoot`. */
+export function removeScratchParentIfEmpty(): void {
+  try {
+    fs.rmdirSync(path.join(process.cwd(), '.containment-test-scratch'))
+  } catch {
+    // Children from this or another suite remain; leave the parent in place.
+  }
+}

common/src/testing/index.ts

@@ -53,6 +53,21 @@ export {
 } from './fixtures/agent-runtime'
 export type { TestAgentRuntimeParams } from './fixtures/agent-runtime'
 
+// ============================================================================
+// Containment Test Fixtures
+// ============================================================================
+
+/**
+ * Filesystem fixtures for suites that create directories outside the project
+ * root and every OS temp root (see `makeOutsideRoot` for why).
+ */
+export {
+  cleanupOutsideRoots,
+  makeOutsideRoot,
+  outsideRootsUsable,
+  removeScratchParentIfEmpty,
+} from './fixtures/containment-fixtures'
+
 // ============================================================================
 // Error Utilities
 // ============================================================================