Files
zero/internal/tools/read_exclusions.go
KRATOS 671af594fb feat(sandbox): harden internal/sandbox — re-entrancy guard, SOCKS5 egress, sandbox-aware search, fine-grained path lists (#199)
* feat(sandbox): harden internal/sandbox with four opt-in capabilities

All four are off by default and fail closed; existing behavior is unchanged
when the new policy fields are empty / ZERO_SANDBOXED is unset.

1. Re-entrancy guard — ZERO_SANDBOXED=1 marks every wrapped command; a nested
   BuildCommandPlan returns a pass-through plan (no double bwrap/sandbox-exec,
   no second egress proxy). New: EnvSandboxed, IsAlreadySandboxed().

2. SOCKS5 egress — the scoped-egress proxy also serves SOCKS5 CONNECT on a
   second loopback port through the SAME authorize()/domainAllowed() gate.
   ProxyEnvWithSocks points ALL_PROXY at socks5://<addr>; HTTP(S)_PROXY stay on
   the HTTP listener; the sandbox-exec profile allows both proxy ports.
   Bubblewrap scoped still collapses to deny.

3. Sandbox-aware search — ReadExclusionGlobs(policy, scope) returns ripgrep
   --glob '!<pat>' for DenyRead subtrees; grep/glob skip read-denied subtrees
   at walk time via Engine.ReadPathExcluded/ReadDirExcluded when sandboxed.

4. Fine-grained path lists — Policy.AllowRead/DenyRead/AllowWrite/DenyWrite.
   Read = deny-then-allow (more-specific AllowRead re-includes); Write =
   DenyWrite > workspace/scope > AllowWrite > deny. AllowWrite is reflected in
   the OS write binds; DenyWrite is emitted as sandbox-exec deny rules. Each
   entry is home-expanded, made absolute, and symlink-resolved.

Tests: internal/sandbox/{reentrancy,egress_socks,pathlists}_test.go and
internal/tools/read_exclusions_test.go.
Gates: gofmt, go vet, go build, go test (incl. -race), staticcheck (no new
findings), govulncheck (0), deadcode -test=false (no new unreachable funcs).

* sandbox: address review — SOCKS5 method negotiation + cache read-exclusion roots

- egress_socks: respect SOCKS5 method negotiation (RFC 1928). The proxy now
  inspects the client's offered methods and replies 0xFF ("no acceptable
  methods") when no-auth was not offered, instead of forcing no-auth.
- pathlists/engine: resolve DenyRead/AllowRead roots ONCE per search walk via a
  new ReadExclusions matcher (Engine.ReadExclusions), replacing the per-path
  ReadPathExcluded/ReadDirExcluded that re-ran Abs/EvalSymlinks for every visited
  path during a grep/glob walk.

Re-entrancy guard left as-is by design: ZERO_SANDBOXED is an internal marker set
only on commands zero wraps; cross-process re-entrancy detection legitimately
relies on the inherited env, and a manually-exported flag is a self-opt-out, not
an external bypass.

Tests: SOCKS no-acceptable-methods rejection; nested-AllowRead descent via the
matcher. Gates: gofmt, vet, build, test (incl. -race), staticcheck (no new
findings), govulncheck (0), deadcode (no new unreachable funcs).

* sandbox: fix windows smoke test + symlink-resolve DirExcluded prefix

- pathlists_test: TestSandboxExecProfileEmitsDenyWriteRule now builds the
  expected deny clause via sandboxProfileString(secret), so it matches the
  profile's own escaping (Windows doubles backslashes). Fixes the
  windows-latest smoke failure; macOS/Linux unaffected (slash paths, no-op
  escaping).
- pathlists: ReadExclusions.DirExcluded now EvalSymlinks-resolves the candidate
  path before the nested-AllowRead prefix check, matching the resolution applied
  to allowRoots — otherwise a denied dir reached THROUGH a symlink fails to match
  a nested AllowRead root and is wrongly skipped, dropping a re-included subtree.
  Added TestDirExcludedResolvesSymlinkPrefix (skips where symlinks are
  unavailable).

Gates: gofmt, vet, build, test (incl. -race), staticcheck (no new findings),
govulncheck (0), deadcode (no new unreachable funcs).

* sandbox: strengthen re-entrancy guard with a corroborating backend marker

Addresses review feedback that the re-entrancy guard trusted a single ambient
env var. IsAlreadySandboxed now requires BOTH correlated markers that
sandboxEnvironment sets together on every wrapped command — ZERO_SANDBOXED=1 AND
a non-empty ZERO_SANDBOX_BACKEND (new EnvSandboxBackend const) — so a lone
stray/hand-exported ZERO_SANDBOXED=1 no longer forces an unsandboxed
pass-through. Pass-through (direct) plans set neither marker, so genuine nesting
still detected and double-wrapping still avoided.

Tests updated: TestIsAlreadySandboxed now asserts each marker alone is
insufficient and both together are required; the re-entrancy and wrapping plan
tests set/clear both markers explicitly.

Gates: gofmt, vet, build, test (incl. -race), staticcheck (no new findings),
govulncheck (0), deadcode (no new unreachable funcs).

* sandbox: don't subject first-party web tools to the network policy by default

The sandbox was reported as too strict: web_search (and web_fetch) were blocked
with "network access is disabled by the sandbox policy" because the default
policy is network=deny and the in-process web tools were gated by the same
NetworkHostAllowed check as the sandboxed shell's egress.

That network policy exists to confine the sandboxed SHELL's egress, which these
in-process tools don't use. New Policy.EnforceToolNetwork (off by default) makes
NetworkHostAllowed exempt web_search/web_fetch unless an operator opts in. The
sandboxed-shell egress decision (Evaluate / effectiveNetworkMode) is unchanged,
and the tools keep their own SSRF/private-IP, port, redirect, and redaction
safeguards. Set EnforceToolNetwork to hold the tools to the allow/scoped/deny
policy as before.

Tests: engine default-exempt + enforced-table cases; web_search default-allows
and enforced-when-flag; web_fetch enforced cases set the flag. Gates: gofmt, vet,
build, test (incl. -race), staticcheck (no new findings), govulncheck (0),
deadcode (no new unreachable funcs).

* sandbox: re-review fixes — disabled policy inert; path lists independent of EnforceWorkspace

- engine: ReadExclusions() / ReadExclusionGlobs() now return inert results under
  ModeDisabled, so a disabled policy never filters grep/glob (parity with
  Evaluate, which already allows everything when disabled).
- engine/pathlists: the fine-grained path lists (DenyRead/DenyWrite, with
  AllowRead/AllowWrite) now apply in Evaluate whenever the sandbox is enforcing,
  independent of EnforceWorkspace — matching the grep/glob path that already
  honors DenyRead directly. The workspace boundary (scope.validate) stays gated
  by EnforceWorkspace via a param threaded through validatePathWithPolicy /
  validateWritePath; DenyWrite wins regardless.
- reentrancy_test: TestSandboxEnvironmentMarksSandboxed now asserts BOTH
  correlated markers (ZERO_SANDBOXED=1 and ZERO_SANDBOX_BACKEND) so dropping
  either would fail the test.

Tests added: path lists enforced with EnforceWorkspace off; ReadExclusions inert
under ModeDisabled. Gates: gofmt, vet, build, test (incl. -race), staticcheck
(no new findings), govulncheck (0), deadcode (no new unreachable funcs).

* sandbox: enforce path lists even without a workspace root

Re-review follow-up: the Evaluate path-validation loop was still guarded by
request.WorkspaceRoot != "", so an engine built without a workspace root skipped
DenyRead/DenyWrite/AllowRead/AllowWrite even for absolute paths. The loop now
runs unconditionally; the workspace boundary (scope.validate) stays gated on
having a root (enforceWorkspace = EnforceWorkspace && WorkspaceRoot != ""), while
the path lists match absolute paths regardless. An unanchorable relative path
(no workspace root) fails closed when anything is configured to enforce, and is a
no-op otherwise (unchanged from prior behavior).

Test: TestEvaluatePathListsWithoutWorkspaceRoot. Gates: gofmt, vet, build, test
(incl. -race), staticcheck (no new findings), govulncheck (0), deadcode (no new
unreachable funcs).

* sandbox: base no-workspace fail-closed on normalized (enforceable) path lists

policyHasPathLists now resolves the lists (resolvePolicyPaths/
resolveWriteRootPaths) instead of counting raw config, matching how the rest of
the file normalizes them. A typo or non-existent entry (dropped during
resolution) therefore no longer makes validatePathWithPolicy fail-close every
relative request when there is no workspace root.

Gates: gofmt, vet, build, test (incl. -race), staticcheck (no new findings),
govulncheck (0), deadcode (no new unreachable funcs).
2026-06-14 17:37:40 +05:30

32 lines
1.2 KiB
Go

package tools
import "github.com/Gitlawb/zero/internal/sandbox"
// readExcluder skips read-denied paths (the sandbox DenyRead policy) during a
// search walk. The zero value (both funcs nil) excludes nothing, so a
// non-sandboxed search behaves exactly as before — the exclusions are opt-in and
// only ever REMOVE results, never add them.
type readExcluder struct {
file func(string) bool
dir func(string) bool
}
func (e readExcluder) fileExcluded(path string) bool { return e.file != nil && e.file(path) }
func (e readExcluder) dirExcluded(path string) bool { return e.dir != nil && e.dir(path) }
// sandboxReadExcluder builds a readExcluder from a sandbox engine's DenyRead
// policy. The engine's read-exclusion matcher resolves the policy paths ONCE here
// (not per visited path), and the closures reuse it for the whole walk. A nil
// engine or an inactive (no DenyRead) policy yields the no-op excluder, so the
// search tools keep their pre-sandbox behavior.
func sandboxReadExcluder(engine *sandbox.Engine) readExcluder {
if engine == nil {
return readExcluder{}
}
rx := engine.ReadExclusions()
if !rx.Active() {
return readExcluder{}
}
return readExcluder{file: rx.PathExcluded, dir: rx.DirExcluded}
}