feat(security): SafeProvider boundary wrapper (Wave 1) #1

Merged

vikingowl merged 3 commits from feat/security-wave1-safeprovider into main

2026-05-19 22:40:47 +02:00

vikingowl commented

2026-05-19 22:40:20 +02:00

Owner

Summary

Closes the four firewall-bypass call sites flagged in the 2026-05-19 external audit by introducing a security.SafeProvider decorator at the provider boundary. Until this PR, only the engine's buildRequest() ran Firewall.ScanOutgoingMessages / ScanSystemPrompt; SLM classifier, summarizer, routerStreamer, and prompt hooks all sent raw payloads to providers.

After this PR, every `provider.Provider` registered with the router (or handed to a non-engine consumer) is wrapped with `security.SafeProvider`. The wrapper resolves the active firewall via `FirewallRef` (an `atomic.Pointer[Firewall]`), which lets us preserve the current init order — wrappers are installed before `NewFirewall` runs, and become active the moment `fwRef.Set(fw)` fires. Pass-through is the safe default.

Plan: `docs/superpowers/plans/2026-05-19-security-wave1-safeprovider.md`

What's wrapped

Primary provider arm (limitedProvider)
Discovered local models (RegisterDiscoveredModels factory)
CLI agent arms (subprocprov.New)
Background-discovery factory
SLM arm + classifier transport (single wrap, reused for both)
Summarizer (gnomactx.NewSummarizeStrategy)
engine.Config.Provider — belt-and-suspenders so a future engine path that bypasses `buildRequest` still routes through the firewall

routerStreamer and hook PromptExecutor inherit the fix because they dispatch through `router.Stream` → `arm.Provider.Stream`.

Elf engines inherit wrapped arms via `elf.Manager.Spawn`. `SpawnWithProvider` is dead code (no callers).

Out of scope (deferred)

Wave 2 — incognito coherence (forced-arm + local-only, unconditional persist, TUI state, file perms)
Wave 3 — scanner + path hygiene (PEM block, strict entropy, canonical AllowedPaths, MCP policy, fs.grep symlink follow)
PostToolUse hook ordering — needs a design decision on raw vs. redacted contract before implementation

Engine reconciliation

`engine.buildRequest()` still scans inline. The plan keeps this for one release as belt-and-suspenders; redaction is idempotent so the second pass is a no-op. Will revisit removing the engine-level scan once telemetry shows the boundary holds.

Test plan

`go test ./...` green
`go test -race ./...` green
`golangci-lint run ./...` — 0 issues
`go build ./...` clean
Unit tests cover SafeProvider pass-through, scanning, and concurrent Set/Get under race detector
Integration tests verify end-to-end redaction via router.Stream, and the pre-Set / post-Set transition behaves as documented

## Summary Closes the four firewall-bypass call sites flagged in the 2026-05-19 external audit by introducing a `security.SafeProvider` decorator at the provider boundary. Until this PR, only the engine's `buildRequest()` ran `Firewall.ScanOutgoingMessages` / `ScanSystemPrompt`; SLM classifier, summarizer, routerStreamer, and prompt hooks all sent raw payloads to providers. After this PR, every \`provider.Provider\` registered with the router (or handed to a non-engine consumer) is wrapped with \`security.SafeProvider\`. The wrapper resolves the active firewall via \`FirewallRef\` (an \`atomic.Pointer[Firewall]\`), which lets us preserve the current init order — wrappers are installed *before* \`NewFirewall\` runs, and become active the moment \`fwRef.Set(fw)\` fires. Pass-through is the safe default. Plan: \`docs/superpowers/plans/2026-05-19-security-wave1-safeprovider.md\` ## What's wrapped - Primary provider arm (limitedProvider) - Discovered local models (RegisterDiscoveredModels factory) - CLI agent arms (subprocprov.New) - Background-discovery factory - SLM arm + classifier transport (single wrap, reused for both) - Summarizer (gnomactx.NewSummarizeStrategy) - engine.Config.Provider — belt-and-suspenders so a future engine path that bypasses \`buildRequest\` still routes through the firewall routerStreamer and hook PromptExecutor inherit the fix because they dispatch through \`router.Stream\` → \`arm.Provider.Stream\`. Elf engines inherit wrapped arms via \`elf.Manager.Spawn\`. \`SpawnWithProvider\` is dead code (no callers). ## Out of scope (deferred) - Wave 2 — incognito coherence (forced-arm + local-only, unconditional persist, TUI state, file perms) - Wave 3 — scanner + path hygiene (PEM block, strict entropy, canonical AllowedPaths, MCP policy, fs.grep symlink follow) - PostToolUse hook ordering — needs a design decision on raw vs. redacted contract before implementation ## Engine reconciliation \`engine.buildRequest()\` still scans inline. The plan keeps this for one release as belt-and-suspenders; redaction is idempotent so the second pass is a no-op. Will revisit removing the engine-level scan once telemetry shows the boundary holds. ## Test plan - [x] \`go test ./...\` green - [x] \`go test -race ./...\` green - [x] \`golangci-lint run ./...\` — 0 issues - [x] \`go build ./...\` clean - [x] Unit tests cover SafeProvider pass-through, scanning, and concurrent Set/Get under race detector - [x] Integration tests verify end-to-end redaction via router.Stream, and the pre-Set / post-Set transition behaves as documented

vikingowl added 3 commits 2026-05-19 22:40:20 +02:00

feat(security): add SafeProvider boundary wrapper (W1-1) 3f2b159ca1

Introduces internal/security/SafeProvider — a provider.Provider decorator
that scans outgoing messages and the system prompt through the firewall
before delegating to the inner provider. Tool-result redaction stays in
the engine because it needs per-tool context the boundary lacks.

FirewallRef provides a late-binding atomic.Pointer[Firewall] so the
wrapper can be installed before NewFirewall runs in main. A nil or
unset ref makes SafeProvider a pass-through — preserves the current
init order without lock contention or panics.

Wave 1 of the post-audit hardening plan
(docs/superpowers/plans/2026-05-19-security-wave1-safeprovider.md).
Closes the architectural critique that secret scanning only ran inside
engine.buildRequest(), leaving SLM/summarizer/hook/routerStreamer paths
to send raw payloads. This commit only ships the wrapper; W1-2 and W1-3
will wire it through main and the four bypass sites.

feat(security): wire SafeProvider into all provider sites (W1-2/3/4) c932f1e307

Construct security.FirewallRef early in main() and Set it immediately
after security.NewFirewall returns. Wrap every provider that may be
called outside engine.buildRequest():

  - primary provider arm (limitedProvider)
  - discovered local models (RegisterDiscoveredModels factory)
  - CLI agent arms (subprocprov.New)
  - background-discovery factory (StartDiscoveryLoop)
  - SLM arm + classifier transport
  - summarizer (gnomactx.NewSummarizeStrategy)

routerStreamer and hook PromptExecutor inherit redaction automatically
once every router arm is wrapped — they dispatch through router.Stream
→ arm.Provider.Stream.

engine.Config.Provider stays raw because the engine still scans inline
at buildRequest(); per the Wave 1 plan, removing that scan is deferred
one release as belt-and-suspenders.

Integration tests in internal/security/integration_test.go verify the
boundary end-to-end: a router arm wrapped with WrapProvider redacts an
'sk-ant-...' literal before the inner provider sees it, and the
pre-Set / post-Set transition works as documented (pass-through until
the FirewallRef has a Firewall installed).

feat(security): wrap engine.Config.Provider + SetProvider doc (W1 follow-up) eb3f3d7134

Advisor flagged that engine.Config.Provider stayed raw, so the safety
property was 'every call goes through buildRequest' instead of the
stronger 'every Stream call routes through a SafeProvider.' Wrap it
even though buildRequest still scans inline — at worst this costs one
extra idempotent scan pass; it removes the 'someone adds a fifth engine
Stream site that skips buildRequest' failure mode.

Engine.SetProvider gets a doc comment establishing the wrap contract
for callers. No active callers today, but documenting it now prevents
the future bypass.

Confirmed elf engines inherit the wrap automatically:
  - elf.Manager.Spawn passes arm.Provider (already *SafeProvider after
    W1-3a)
  - elf.Manager.SpawnWithProvider has no callers — dead code path

Added the Wave 1 plan to TODO.md under active plans.

vikingowl merged commit be6ef77cc5 into main

2026-05-19 22:40:47 +02:00

vikingowl referenced this pull request

2026-05-19 22:58:08 +02:00

feat(security): incognito coherence (Wave 2) #2

vikingowl referenced this pull request

2026-05-19 23:16:41 +02:00

feat(security): scanner + path hygiene (Wave 3) #3

vikingowl referenced this issue from a commit

2026-05-20 03:27:47 +02:00

Merge pull request 'feat(security): SafeProvider boundary wrapper (Wave 1)' (#1) from feat/security-wave1-safeprovider into main

vikingowl referenced this issue from a commit