mirror of
https://github.com/Twigpine/zero.git
synced 2026-11-12 08:41:32 -10:00
* 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).
100 lines
3.1 KiB
Go
100 lines
3.1 KiB
Go
package tools
|
|
|
|
import (
|
|
"context"
|
|
"os"
|
|
"path/filepath"
|
|
"strings"
|
|
"testing"
|
|
|
|
"github.com/Gitlawb/zero/internal/sandbox"
|
|
)
|
|
|
|
// denyReadFixture builds a workspace with an ordinary file and a secret subtree,
|
|
// returning the resolved workspace root and a sandbox engine whose DenyRead
|
|
// covers the secret dir.
|
|
func denyReadFixture(t *testing.T) (string, *sandbox.Engine) {
|
|
t.Helper()
|
|
ws, err := filepath.EvalSymlinks(t.TempDir())
|
|
if err != nil {
|
|
t.Fatalf("EvalSymlinks: %v", err)
|
|
}
|
|
if err := os.WriteFile(filepath.Join(ws, "main.go"), []byte("package main\n"), 0o600); err != nil {
|
|
t.Fatalf("write main.go: %v", err)
|
|
}
|
|
secret := filepath.Join(ws, "secret")
|
|
if err := os.MkdirAll(secret, 0o755); err != nil {
|
|
t.Fatalf("mkdir secret: %v", err)
|
|
}
|
|
if err := os.WriteFile(filepath.Join(secret, "creds.go"), []byte("package main\n"), 0o600); err != nil {
|
|
t.Fatalf("write creds.go: %v", err)
|
|
}
|
|
|
|
scope, err := sandbox.NewScope(ws, nil)
|
|
if err != nil {
|
|
t.Fatalf("NewScope: %v", err)
|
|
}
|
|
engine := sandbox.NewEngine(sandbox.EngineOptions{
|
|
WorkspaceRoot: ws,
|
|
Policy: sandbox.Policy{
|
|
Mode: sandbox.ModeEnforce,
|
|
EnforceWorkspace: true,
|
|
DenyRead: []string{secret},
|
|
},
|
|
Scope: scope,
|
|
})
|
|
return ws, engine
|
|
}
|
|
|
|
func TestGrepSkipsDenyReadSubtree(t *testing.T) {
|
|
ws, engine := denyReadFixture(t)
|
|
tool, ok := NewGrepTool(ws).(sandboxAwareTool)
|
|
if !ok {
|
|
t.Fatal("grep tool must be sandbox-aware")
|
|
}
|
|
args := map[string]any{"pattern": "package main", "output_mode": "files_with_matches"}
|
|
|
|
// Sandboxed: the DenyRead subtree must be excluded from results.
|
|
sandboxed := tool.RunWithSandbox(context.Background(), args, engine)
|
|
if sandboxed.Status != StatusOK {
|
|
t.Fatalf("grep failed: %s", sandboxed.Output)
|
|
}
|
|
if !strings.Contains(sandboxed.Output, "main.go") {
|
|
t.Fatalf("grep must still match the non-denied file, got:\n%s", sandboxed.Output)
|
|
}
|
|
if strings.Contains(sandboxed.Output, "creds.go") {
|
|
t.Fatalf("grep must NOT surface a DenyRead file, got:\n%s", sandboxed.Output)
|
|
}
|
|
|
|
// Without a sandbox, the same search includes the secret file (default behavior).
|
|
plain := NewGrepTool(ws).Run(context.Background(), args)
|
|
if !strings.Contains(plain.Output, "creds.go") {
|
|
t.Fatalf("non-sandboxed grep should include the secret file, got:\n%s", plain.Output)
|
|
}
|
|
}
|
|
|
|
func TestGlobSkipsDenyReadSubtree(t *testing.T) {
|
|
ws, engine := denyReadFixture(t)
|
|
tool, ok := NewGlobTool(ws).(sandboxAwareTool)
|
|
if !ok {
|
|
t.Fatal("glob tool must be sandbox-aware")
|
|
}
|
|
args := map[string]any{"pattern": "**/*.go"}
|
|
|
|
sandboxed := tool.RunWithSandbox(context.Background(), args, engine)
|
|
if sandboxed.Status != StatusOK {
|
|
t.Fatalf("glob failed: %s", sandboxed.Output)
|
|
}
|
|
if !strings.Contains(sandboxed.Output, "main.go") {
|
|
t.Fatalf("glob must still match the non-denied file, got:\n%s", sandboxed.Output)
|
|
}
|
|
if strings.Contains(sandboxed.Output, "creds.go") {
|
|
t.Fatalf("glob must NOT surface a DenyRead file, got:\n%s", sandboxed.Output)
|
|
}
|
|
|
|
plain := NewGlobTool(ws).Run(context.Background(), args)
|
|
if !strings.Contains(plain.Output, "creds.go") {
|
|
t.Fatalf("non-sandboxed glob should include the secret file, got:\n%s", plain.Output)
|
|
}
|
|
}
|