…ion-mode-redirects

…ns [PR C]

In remediation mode, output redirections (>, >>, 2>, &>, &>>) open through
the AllowedPaths sandbox instead of being blocked at parse time. /dev/null
is still short-circuited to io.Discard. <> stays blocked in all modes.

- interp/validate.go: validateNode/validateRedirect accept remediationMode bool;
  file-target output redirect cases return nil (pass) when true
- interp/api.go: pass r.remediationMode to validateNode
- interp/runner_redir.go: in remediation mode, open file targets via the sandbox
  with O_WRONLY|O_CREATE|O_TRUNC (>) or O_WRONLY|O_CREATE|O_APPEND (>>);
  add rejectNonRegularRedirectTarget guard to prevent hangs on FIFOs
- analysis/symbols_interp.go: add os.O_APPEND, O_CREATE, O_TRUNC, O_WRONLY
  and io/fs.ModeType to the allowed symbol list
- tests/scenarios_test.go: add remediation_mode field to input struct
- tests/scenarios/shell/redirections/file_target/: 8 new scenarios covering
  truncate, overwrite, append, 2>, &>, &>>, sandbox-blocked, and > file 2>&1

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

…ionMode() with Mode()

Makes the Go API mirror the CLI's --mode flag: callers now write
interp.Mode(interp.ModeRemediation) instead of the asymmetric
interp.RemediationMode().

- type Mode string → type ExecutionMode string (frees "Mode" as a
  function name)
- RemediationMode() RunnerOption → Mode(m ExecutionMode) RunnerOption
  (validates the value, rejects unknown modes)
- cmd/rshell/main.go: interp.Mode(mode) type-conv → interp.ExecutionMode(mode);
  always pass interp.Mode(opts.mode) so read-only is explicit too
- tests/scenarios_test.go: updated call site

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

Proves the mode gate (not just the sandbox) blocks writes: AllowedPaths
is configured so the sandbox would permit writes, but without remediation
mode the validator rejects >, >>, and &> at parse time (exit 2).

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

Resolve 5 conflicts:
- allowedpaths/sandbox.go: take PR C's readOnly default + SetWritable() +
  tightened allowedOpenFlags (removes O_RDWR/O_EXCL)
- allowedpaths/sandbox_test.go: take PR C's TestSandboxDefaultReadOnly and
  updated SetWritable-gated write tests
- analysis/symbols_allowedpaths.go: drop O_EXCL/O_RDWR (removed from sandbox)
- interp/api.go: keep PR D's Mode(ExecutionMode) API; add SetWritable() call
  in New() when remediationMode is true
- README.md: keep PR D's AllowedPaths paragraph (includes remediation mode docs)

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

os.O_APPEND, os.O_CREATE, os.O_TRUNC, os.O_WRONLY, and io/fs.ModeType
were added to the interpAllowedSymbols ceiling but omitted from
interpPerModeSymbols["read-only"]. runner_redir.go (not a _remediation.go
file) uses all five, so TestInterpPerModeSymbols failed.

Add them to the "read-only" list. The runtime guard (r.remediationMode)
still prevents actual writes in read-only mode; these are flag constants
whose capability gate is allowedpaths.Sandbox.Open.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

Move write-capable code (openWriteRedirect, openWriteAllRedirect,
rejectNonRegularRedirectTarget) out of runner_redir.go and into a new
_remediation.go file, so the write-flag symbols (os.O_WRONLY, os.O_CREATE,
os.O_TRUNC, os.O_APPEND, io/fs.ModeType, syntax.RedirOperator) can be listed
in interpPerModeSymbols["remediation"] only — not in "read-only". This restores
the static guarantee that write-capable symbols cannot appear in non-remediation
interp/ files.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

rejectNonRegularRedirectTarget previously returned nil when r.sandbox == nil,
silently skipping the check. Opening a FIFO with O_WRONLY blocks until a reader
connects, so a sandboxless remediation-mode runner would hang for up to 30s on
any named-pipe redirect target.

Extract a statFileMode helper that uses r.sandbox.Stat when a sandbox is
present, and falls back to os.Stat otherwise. The check now always runs,
regardless of whether AllowedPaths is configured.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

Covers the one file-target redirect operator that had no dedicated test:
stderr append (2>>). Follows the same pattern as append.yaml and
stderr_redirect.yaml — appends stderr twice and verifies both lines appear.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

Extract the repeated `redirectTargetIsDevNull(rd) || remediationMode`
condition into a redirectAllowed helper, removing the duplication across
all four write-redirect cases in validateRedirect (suggested by AlexandreYang).

Add clobber.yaml to cover the >| operator, the one file-target redirect
with no dedicated scenario.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

On Windows, Go's filepath treats /tmp/evil.txt as the relative path
tmp\evil.txt rather than an absolute path. The open therefore fails with
"no such file or directory" before the sandbox can reject it with
"permission denied". Add stderr_contains_windows to accept the
Windows-specific error while keeping the stricter check on Linux/macOS.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

SHELL_FEATURES.md still referenced interp.RemediationMode() which was
renamed to interp.Mode(interp.ModeRemediation) in this PR. Fix the doc.

sandbox.Open() returns ErrPermission immediately when called on a nil
receiver (sandbox.go:368-370), so r.open() always fails before touching
a FIFO when r.sandbox == nil. The os.Stat fallback in statFileMode was
therefore dead code — a nil sandbox already prevents any blocking open.
Replace it with an early return of os.ErrNotExist so the FIFO type-check
is skipped (the subsequent open failure surfaces the real error), and
remove os.Stat from the symbol allowlists.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

PR #524 reviewer explicitly asked for type Mode with ModeReadOnly and
ModeRemediation constants. PR #526 had renamed the type to ExecutionMode
to avoid a naming conflict with the Mode() RunnerOption function, but
that went against the prior review feedback.

Restore type Mode string (as requested in PR #524) and rename the
RunnerOption from Mode() to WithMode(), which reads naturally at the
call site (interp.WithMode(interp.ModeRemediation)) and eliminates the
awkward inversion where the setter and the type had swapped names.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

Adds `truncate -s SIZE [-c] FILE...` as a new builtin that is only
wired when the interpreter is in remediation mode. The primary use
case is zeroing log files in place to reclaim disk space without
breaking processes that hold the file open.

Key design decisions:
- Sandbox.Truncate opens with O_NONBLOCK so FIFOs return ENXIO
  immediately instead of blocking, and fstats the fd (not the path)
  before ftruncate to close the TOCTOU race window.
- The callCtx.Truncate field is nil outside remediation mode, making
  the mode guard a nil-pointer check rather than a separate flag.
- Relative-size modifiers (+/-/<>//%/) are rejected with an explicit
  hint rather than silently treated as invalid numbers.
- Z/Y/R/Q suffixes (zetta+) are rejected; their multipliers exceed
  int64.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

- Switch readonly_mode and sandbox_blocked scenarios from stderr_contains
  to exact expect.stderr (AGENTS.md prefers exact match when deterministic)
- Add comment to Sandbox.Truncate explaining why O_NONBLOCK bypasses
  allowedOpenFlags (hardcoded, not user-derived — different risk model)
- Add comment to parseSize explaining ParseInt vs ParseUint choice
- Add TestTruncatePentestFIFOTarget (unix-only) exercising the O_NONBLOCK
  FIFO-blocking guard, which is a key security property of Sandbox.Truncate

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

Sandbox.Open checks s == nil and s.readOnly before proceeding; Sandbox.Truncate
did not, so a direct library caller on a default-mode (read-only) sandbox could
still write. Mirror the same guards so Truncate is consistent with Open as a
public API.

The interp wiring already guards against this at the runner level (callCtx.Truncate
is nil unless remediationMode && sandbox != nil), but the sandbox itself should
enforce its own invariants.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

Extend the static analysis system with a new per-builtin check that
enforces which CallContext function fields each builtin is permitted to
access directly.

Key security invariants now statically verified:
- Only the "truncate" builtin may access callCtx.Truncate (the sole
  write-capable CallContext field).
- Only "find" and "xargs" may access RunCommand/RunCommandWithStdin.
- Only "cd" may access ChangeDir.
- Only "read" may access SetVar.

Implementation:
- callCtxAllFields (symbols_builtins.go): the complete set of tracked
  CallContext function-typed fields (plain data fields like Stdin/Now/Proc
  are excluded as they carry no elevated capability).
- builtinPerCommandCallContextFields (symbols_builtins.go): per-builtin
  allowlists, backfilled from actual source analysis.
- checkFileCallCtxFields (structural.go): AST walker that detects depth-1
  selector expressions (bareIdent.Field) where bareIdent is not a package
  import alias and Field is a known CallContext field.
- checkCallCtxFields (symbols_test.go): harness that validates allowlist
  integrity (no unknown fields) and per-builtin file compliance.
- TestBuiltinCallContextFields + 5 verification tests covering: clean
  pass, unauthorized Truncate access, unlisted field, field not in global
  set, and missing builtin entry.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

…er detection

The depth-1 check previously flagged any bare-ident selector whose field
name matched a tracked CallContext field, regardless of the variable's
actual type. This caused a false-positive in du: dh.ReadDir(1) (where dh
is an fs.ReadDirFile) was detected as a CallContext.ReadDir access, forcing
ReadDir into du's allowlist even though du never accesses callCtx.ReadDir.

Fix: replace the import-alias exclusion heuristic with precise holder
detection. findCallCtxHolderNames scans each file for identifiers statically
known to hold a *CallContext value — function parameters and struct fields
typed *CallContext or *<pkg>.CallContext. The depth-1 check now flags only
bare idents in this set; local variables of other types are ignored.

Consequences:
- dh.ReadDir (fs.ReadDirFile) is no longer flagged — ReadDir removed from du
- The false-positive note is removed from the map comment
- fileImportNames is deleted (no longer needed)
- findCallCtxBridgeFields is replaced by findCallCtxHolderNames (superset)
- Both depth-1 and depth-N use the same holders set

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

…ecker

A developer could bypass the checker by aliasing callCtx to a local
variable (cc := callCtx; cc.Truncate) since only declaration-based
names (function params and struct fields typed *CallContext) were in
the holder set.

Fix: findCallCtxLocalAliases scans AssignStmt nodes in each file and
adds local variable names whose RHS is a *CallContext value expression
(bare holder ident or x.bridgeField selector). Iterated to stable so
transitive chains are covered (cc := callCtx; dd := cc).

Local aliases are computed per-file in phase 2 (not merged globally)
so a name aliased in file A does not pollute the holder set for file B.

New TestVerificationCallCtxLocalAlias injects cc := callCtxProbe;
cc.Truncate into cat and confirms detection.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>