Skip to content

feat(hooks): v2 sandbox — env-allowlist + Linux Landlock + SIGTERM grace + buffer cap#109

Open
Fullstop000 wants to merge 6 commits into
masterfrom
worktree-feat-hook-sandbox-v2
Open

feat(hooks): v2 sandbox — env-allowlist + Linux Landlock + SIGTERM grace + buffer cap#109
Fullstop000 wants to merge 6 commits into
masterfrom
worktree-feat-hook-sandbox-v2

Conversation

@Fullstop000
Copy link
Copy Markdown
Owner

@Fullstop000 Fullstop000 commented Jun 2, 2026

Summary

v2 of the hook protocol's security model — closes the unsandboxed gap v1 (#102) deliberately shipped. Four layers:

  1. Env-var allowlist (all platforms, ~80 LOC). Default pass-through: PATH HOME USER LANG LC_ALL TZ. Per-hook env: ["KEY", …] adds to it. Command::env_clear() + explicit cmd.env(k, v) for each. Stops accidental credential exfil.
  2. Linux Landlock filesystem sandbox (~275 LOC + landlock = "0.4"). Per-hook sandbox: bool (default true). Pre-exec hook applies a ruleset that lets the child read its hook folder, /etc/ssl/certs, /usr/lib, /lib, /lib64, /bin, /usr/bin, /sbin, /usr/sbin, /etc/resolv.conf, /dev/urandom, $TMPDIR; write $TMPDIR and /dev/null only. Non-Linux: empty stub, startup warning.
  3. Real SIGTERM-then-SIGKILL grace (~30 LOC). Replaces v1's kill_on_drop-only path. On timeout: libc::kill(pid, SIGTERM) → 1s sleep → child.kill() (SIGKILL). tokio's start_kill actually sends SIGKILL, so the implementation uses libc directly (commented).
  4. Bounded stdout/stderr buffer (~50 LOC, Codex M finding from feat(hooks): external subprocess hook protocol with prompt mutation #102). 1 MiB cap per stream. On overflow: truncate + emit Warning { source: "hook.buffer", … }.

Architecture decision

The original plan (memory [[mini-sandbox-bypass-hardening]]) called for Fullstop000/mini-sandbox. That crate is unpublished on crates.io and ~50 LOC of Landlock wrapper. We use landlock directly for crates.io provenance and to thread a typed SandboxStatus into the tracing span. Functionally equivalent; better supply-chain hygiene.

Sandbox status telemetry

pre_exec can't backchannel from child to parent, so the parent probes the kernel's Landlock ABI once at startup (landlock_create_ruleset(NULL, 0, VERSION)) and reports the expected enforcement level in the ignis.hook.dispatch tracing span. On hosts without Landlock: per-hook warning emitted once per session (cleared by /hooks reload).

Test plan

  • cargo fmt --all -- --check
  • cargo clippy --workspace --all-targets -- -D warnings — 0 warnings
  • cargo test --workspace — 544 passed / 0 failed / 1 ignored
  • cargo build --release
  • Dogfooded on this host (WSL2 kernel 6.6, Landlock ABI V3): real Landlock enforcement confirmed. A hook that writes outside $TMPDIR exits non-zero with [warn]; a sandbox-clean hook works as in v1.
  • Integration test tests/hook_sandbox.rs verifies a hook attempting to write $HOME/leak-from-hook.txt is blocked when sandbox is on, succeeds when off.

Known intermittent flake

hooks::dispatch::tests::exit_zero_empty_stdout_is_passthrough flakes ~3/10 times under full parallel cargo test --workspace due to concurrent fork/exec on shared tempdir scripts (ETXTBSY). Passes deterministically in isolation. Not introduced by this PR — same pattern in v1's hook tests. Worth a serial_test gating PR later.

Documentation

  • docs/usage/hooks.md — replaces the v1 "unsandboxed" warning block with the actual v2 model (threat scoped to network egress; sandbox default-on, opt-out per hook).
  • examples/hooks/translate-en/README.md — example hooks.json now declares env: ["ANTHROPIC_API_KEY", …] and sandbox: true.
  • docs/superpowers/specs/2026-06-03-hook-sandbox-v2-design.md (local-only).

What's still deferred (v3+)

  • Seccomp syscall filter, network egress allowlist, macOS Seatbelt, Windows AppContainer.
  • Per-hook custom Landlock rules in TOML (currently default-or-off).

Fullstop000 added a commit that referenced this pull request Jun 2, 2026
@Fullstop000
docs(changelog): hooks v2 PR #109
d8bf2cf
@Fullstop000
feat(hooks): sandbox + env-allowlist + grace timeout + buffer cap
718532b 
Implements the four-layer v2 sandbox from
docs/superpowers/specs/2026-06-03-hook-sandbox-v2-design.md:

1. Env-var allowlist on every platform. The child sees only
   `PATH HOME USER LANG LC_ALL TZ` from ignis's env, plus the names
   the hook declared in `env: [...]`. Closes the v1 credential-exfil
   gap.

2. Linux Landlock filesystem sandbox installed via `pre_exec` in the
   forked child. Per-hook `sandbox: bool` (default `true`). Allows
   reads from the hook folder + standard lib paths + TLS roots +
   $TMPDIR + /dev/urandom; writes from $TMPDIR and /dev/null only.
   Non-Linux builds compile to a no-op stub returning
   `PlatformUnsupported`; an unsupported kernel emits a one-time
   `hook.sandbox` warning per hook per session, cleared by
   `/hooks reload`.

3. SIGTERM → 1 s grace → SIGKILL on timeout. tokio's
   `Child::start_kill` maps to SIGKILL on Unix, so we deliver SIGTERM
   directly via `libc::kill(pid, SIGTERM)`.

4. 1 MiB-per-stream buffer cap. Surplus bytes are drained and
   discarded so the child can finish writing without blocking on a
   full pipe; a `hook.buffer` Warning event lands when truncation
   happens.

Schema additions on `HookSpec`: `env: Vec<String>` and
`sandbox: bool` (default `true` from JSON via Option-unwrap; default
`false` in-memory via `bool::default()` for test fixtures using
`..HookSpec::default()`).

Tests:
- config: defaults, env list ordering, sandbox false opt-out
- dispatch: env_filter, sigterm_grace, buffer_cap
- sandbox unit: default read/write lists, status labels
- integration (tests/hook_sandbox.rs, Linux only): sandboxed write
  to non-/tmp path blocked; unsandboxed write succeeds
@Fullstop000
docs(hooks): document v2 sandbox; replace v1 unsandboxed warning
2679260 
Updates `docs/usage/hooks.md`:
- Replace the prominent "Hooks run unsandboxed in v1" callout with a
  spec of the four v2 layers (env allowlist, Landlock, SIGTERM grace,
  buffer cap), defaults, and platform behaviour.
- Document `env: [...]` and `sandbox: bool` config fields.
- Update timeout-behaviour paragraph from "SIGKILL via kill_on_drop"
  to "SIGTERM → 1 s grace → SIGKILL".
- Note that `/hooks reload` resets the per-session degradation
  warning suppression.

Updates `examples/hooks/translate-en/README.md`:
- Replace the v1 unsandboxed warning with a one-line v2 description.
- Sample `hooks.json` now declares `env` (the three vars the
  translator actually needs) and `sandbox: true`.

Adds a one-line `[Unreleased]` CHANGELOG entry. PR # is TBD until the
PR opens.
@Fullstop000
fix(hooks): address dual-reviewer findings on v2 sandbox
917cc21 
- pre_exec is now allocation-free: parent pre-builds the Landlock read/write
  path lists and the closure only does syscalls; errors return raw OS codes
  rather than the boxing `io::Error::other`. Closes the async-signal-safety
  hazard both reviewers flagged.
- Bare hook programs (`python3 hook.py`, no parent dir) no longer fall back
  to `hook_folder = /`, which silently disabled read confinement. The hook
  folder is now `Option<PathBuf>` and omitted from reads when absent.
- TMPDIR no longer trusted from env: the sandbox hardcodes `/tmp` + `/var/tmp`
  as the scratch directories. Launching ignis with `TMPDIR=$HOME` no longer
  exposes the home directory to every sandboxed hook.
- HookSpec now has a manual Default with `sandbox: true` (was `false` via
  derived Default), so tests written with `..HookSpec::default()` exercise
  the secure-by-default code path. /dev/zero added to the default read list
  so shell-script hooks that read it (common idiom) still work.
- `SandboxStatus::PlatformUnsupported` now triggers the same once-per-session
  warning as `NotEnforced`, matching the docs.
- Stale strings cleaned up: `/hooks reload` no longer says "run unsandboxed",
  and the `hooks` module doc-comment now describes v2 behaviour.
@Fullstop000 Fullstop000 force-pushed the worktree-feat-hook-sandbox-v2 branch from d8bf2cf to 917cc21 Compare June 2, 2026 17:55
@Fullstop000
docs(hooks): update sandbox module preamble to reflect v2 defaults
8844811
@Fullstop000
refactor(sandbox): promote Landlock primitive to top-level sandbox mod
ad8d88f 
The hook protocol shipped Landlock confinement living under
`ignis/src/hooks/sandbox.rs`, but the same mechanism applies to any
ignis subprocess caller (bash-tool next). Splits along the
mechanism/policy line:

  * `ignis/src/sandbox/mod.rs` (new) — generic: `SandboxStatus`,
    `apply_with_paths(reads, writes)`, the Linux Landlock syscalls.
    Policy-free; takes pre-built path slices.
  * `ignis/src/hooks/sandbox.rs` — hook-specific policy: the default
    read/write path lists (TLS roots, scratch dirs, interpreter
    paths) and a convenience `apply(hook_folder)` wrapper that calls
    into `crate::sandbox::apply_with_paths`.

No behaviour change; tests preserved in their owning module. Future
bash-tool sandbox lives next to its caller and reuses the same
primitive via `use ignis::sandbox::apply_with_paths;`.
@Fullstop000
feat(sandbox): macOS Seatbelt support via sandbox_init()
7d44acc 
Adds a `SandboxPolicy` two-step API (parent builds, child applies) so
the macOS branch can serialise a Seatbelt Scheme profile into a
`CString` ahead of fork — keeping the `pre_exec` closure
allocation-free on both targets. Linux Landlock behaviour is unchanged
(refactored from `apply_with_paths` to the same policy type, free
function preserved for back-compat).

New ignis/src/sandbox/macos.rs declares `sandbox_init` /
`sandbox_free_error` via libc-only `extern "C"` bindings (no new
dependency), translates the same hook default path list into Seatbelt
allow rules, and mirrors `/tmp` → `/private/tmp` and `/var` →
`/private/var` so post-symlink-resolution opens match.

The macOS branch compiles under cfg gating but is unverified on macOS
hardware from this Linux WSL2 host. The runtime behaviour of
sandbox_init against this profile shape needs Mac dogfood before
merge.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@Fullstop000