feat(zerocommands): add typed SandboxGrantSnapshot contract#89
Conversation
The merged permission and verification event contracts added typed
snapshots for config, providers, models, and sessions, but the
persistent sandbox grant store (internal/sandbox/grants.go) still
exposes only sandbox.Grant directly. The TUI render path, the
headless 'zero sandbox' command, and PR/CI automation all need a
typed snapshot so they do not hand-roll map[string]any payloads and
do not accidentally leak grant reasons that contain API keys or
tokens.
This commit adds SandboxGrantSnapshot alongside the existing
ProviderSnapshot, ModelSnapshot, SessionSnapshot, and
SessionTreeSnapshot in internal/zerocommands. The snapshot mirrors
the format of the existing typed contracts:
- ToolName, Decision, MaxAutonomy, ApprovedAt are copied verbatim
from the underlying grant. They are non-secret metadata.
- Reason is run through the standard redaction pipeline before it
is copied, so any secret material the user typed when granting
is masked before the snapshot leaves the runtime.
- All string fields are trimmed. An empty or whitespace-only
Reason becomes an empty Reason in the snapshot so JSON output
omits the field.
Two constructors are provided:
- SandboxGrantSnapshotFromGrant(grant) for a single grant.
- SandboxGrantSnapshots(grants) for a slice. Output is sorted
alphabetically by ToolName so consumers (TUI, headless, JSON
output) see a deterministic ordering. An empty input returns
a non-nil empty slice so JSON output is always '[]' and never
'null'.
A matched/unmatched helper is also added:
- SandboxGrantMatchSnapshotFromLookup(toolName, lookup) pairs the
typed snapshot with a Matched boolean. When the lookup did not
match, the returned snapshot has Matched=false and a nil Grant
pointer, so consumers can render the absence without inspecting
an error or a typed zero value.
Tests
- TestSandboxGrantSnapshotFromGrantRedactsReasonAndTrimsFields:
verifies the secret in the reason is masked and the surrounding
fields are trimmed.
- TestSandboxGrantSnapshotFromGrantEmptyReasonBecomesEmpty:
verifies whitespace-only reasons normalize to the empty string.
- TestSandboxGrantSnapshotsSortsByToolNameAndReturnsEmptySliceForEmptyInput:
verifies alphabetical sort and that nil input returns a non-nil
empty slice whose JSON encoding is '[]'.
- TestSandboxGrantMatchSnapshotFromLookupMatchedAndUnmatched:
verifies the matched and unmatched code paths.
- TestSandboxGrantSnapshotJSONShapeIsStable:
verifies every JSON key is present so downstream consumers can
rely on the shape.
Verification
- go build ./...
- go vet ./...
- go test -count=1 -p 1 ./internal/zerocommands/... \
./internal/sandbox/... ./internal/redaction/...
DRI: Gnanam (runtime core owner per WORK_SPLIT_PRD.md §4).
Connects: 'zero sandbox' command rendering, TUI grants panel, and
PR/CI automation consumers to the persistent grant store without
leaking grant reasons.
Zero automated PR reviewVerdict: No blockers found Blockers
Validation
ScopeHead: This deterministic review checks validation status and basic diff hygiene. A human reviewer still owns product judgment and design quality. |
|
Looking for one thing? Review this PR in Change Stack to search files, summaries, diffs, and code without losing your place. WalkthroughNew module ChangesSandbox Grant Snapshot Export
🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/zerocommands/sandbox_snapshot.go (1)
47-48: ⚡ Quick winTrim enum-backed strings before exporting the snapshot.
DecisionandMaxAutonomyare copied without trimming, so whitespace from persisted/corrupted values can leak into output and break the “trimmed string fields” contract.Suggested patch
return SandboxGrantSnapshot{ ToolName: strings.TrimSpace(grant.ToolName), - Decision: string(grant.Decision), - MaxAutonomy: string(grant.MaxAutonomy), + Decision: strings.TrimSpace(string(grant.Decision)), + MaxAutonomy: strings.TrimSpace(string(grant.MaxAutonomy)), ApprovedAt: strings.TrimSpace(grant.ApprovedAt), Reason: redaction.RedactString(strings.TrimSpace(grant.Reason), redaction.Options{}), }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/zerocommands/sandbox_snapshot.go` around lines 47 - 48, When building the snapshot, trim whitespace from enum-backed fields so persisted/corrupted values don't leak; update the assignments that set Decision and MaxAutonomy (currently using string(grant.Decision) and string(grant.MaxAutonomy)) to use strings.TrimSpace(string(grant.Decision)) and strings.TrimSpace(string(grant.MaxAutonomy)); import strings if needed and keep the rest of the snapshot construction (the grant variable and surrounding export logic) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@internal/zerocommands/sandbox_snapshot.go`:
- Around line 47-48: When building the snapshot, trim whitespace from
enum-backed fields so persisted/corrupted values don't leak; update the
assignments that set Decision and MaxAutonomy (currently using
string(grant.Decision) and string(grant.MaxAutonomy)) to use
strings.TrimSpace(string(grant.Decision)) and
strings.TrimSpace(string(grant.MaxAutonomy)); import strings if needed and keep
the rest of the snapshot construction (the grant variable and surrounding export
logic) unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 39f8dc68-0463-4214-82a3-e7e6b5da76a2
📒 Files selected for processing (2)
internal/zerocommands/sandbox_snapshot.gointernal/zerocommands/sandbox_snapshot_test.go
BlockersNone found. Non-Blocking
Looks Good
|
…cision snapshots (#92) * feat(zerocommands): add typed sandbox policy, risk, violation, and decision snapshots The merged permission and verification event contracts (#82) added typed snapshots for config, providers, models, and sessions, and PR #89 added the typed snapshot for the persistent sandbox grant store, but the live sandbox policy, the live risk classification, the live violation, and the live decision were still exposed as the raw backend structs. The TUI render path, the headless 'zero sandbox policy --json' and 'zero sandbox decide' commands, the audit log, and PR/CI automation all need typed snapshots so they do not hand-roll map[string]any payloads and so they do not have to re-implement the runtime default-mode translation themselves. This commit adds five typed snapshot constructors and a bundled plan snapshot to internal/zerocommands. SandboxPolicySnapshot - mode, network, enforceWorkspace, denyDestructiveShell, and allowPolicyOnlyRunner are copied verbatim from the underlying sandbox.Policy. - effectiveMode is the resolved policy mode. An empty Mode in the input falls back to ModeEnforce, which is the runtime default. Consumers can read EffectiveMode to render the 'what is actually in effect' line without re-implementing the default. - effectiveMode is JSON-omitted when empty so the snapshot shape stays stable for downstream consumers. SandboxRiskSnapshot - level, categories, reason. - categories is copied and sorted alphabetically so the JSON output is deterministic. - reason is trimmed. SandboxViolationSnapshot - code, toolName, action, path, reason, recoverable. - The pointer-returning helper returns nil for a nil input so callers can chain it through SandboxDecisionSnapshot without nil checks at the call site. SandboxBackendSnapshot - name, available, platform, fallback, commandWrapping, nativeIsolation, executable, message. - executable and message are trimmed. SandboxPlanSnapshot - bundles policy, backend, restrictions, and workspaceRoot into one payload so 'zero sandbox policy --json' can return one typed result describing the full sandbox posture. - each entry of restrictions is trimmed, whitespace-only entries are dropped, and the result is sorted alphabetically. - workspaceRoot is trimmed and JSON-omitted when empty. SandboxDecisionSnapshot - action, reason, risk, grantMatched, grant, violation. - The optional grant pointer is converted with SandboxGrantSnapshotFromGrant (from PR #89) so the JSON shape matches the persistent-grant snapshot exactly. - The optional violation pointer is converted with SandboxViolationSnapshotFromViolation so the snapshot always carries the same shape regardless of the decision outcome. - When grant or violation is nil, the field is JSON-omitted. Tests - TestSandboxPolicySnapshotFromPolicyFillsEffectiveMode: verifies EffectiveMode is enforced for an empty Mode, set to ModeDisabled for an explicit disabled mode, and preserved for an explicit enforced mode. - TestSandboxRiskSnapshotFromRiskSortsCategoriesAndTrimsReason: verifies alphabetical sort, trimmed reason, and that a nil Categories input produces a nil output (no spurious empty slice). - TestSandboxViolationSnapshotFromViolationHandlesNilAndTrims: verifies the nil-input contract and the trim behaviour for every string field. - TestSandboxBackendSnapshotFromBackendCopiesAllFields: verifies the boolean and string fields round-trip and the trim behaviour for the executable and message. - TestSandboxPlanSnapshotFromPlanSortsRestrictionsAndTrimsRoot: verifies alphabetical sort, per-entry trim, and workspaceRoot trim. - TestSandboxDecisionSnapshotFromDecisionAllowBranchHasNoViolation: verifies the allow path has no grant and no violation. - TestSandboxDecisionSnapshotFromDecisionPersistentDenyCarriesGrantAndViolation: verifies the deny path carries both the resolved grant and the violation, and that the grant snapshot matches the persistent-grant snapshot shape from PR #89. - TestSandboxDecisionSnapshotJSONShapeIsStable: verifies the stable keys and the nil-pointer omission. - TestSandboxPolicySnapshotJSONOmitsEmptyEffectiveMode: verifies the omitempty tag. Verification - go build ./... - go vet ./... - go test -count=1 -p 1 ./internal/zerocommands/... \ ./internal/sandbox/... ./internal/redaction/... DRI: Gnanam (runtime core owner per WORK_SPLIT_PRD.md §4). Closes the typed-snapshot gap for the live sandbox surface that the TUI render path, the 'zero sandbox' headless commands, the audit log, and PR/CI automation consume. * fix(npmwrapper): make copyWrapperFixture create package.json with type:module This ensures the isolated wrapper .js fixture is always treated as ESM (matching real package installs). Previously, without it, Node treated the .js as CJS on some platforms/runners (especially Windows), causing SyntaxError before the missing-native console.error path. This was causing TestNodeWrapperReportsMissingNativeBinary to get empty output + timeout on windows-latest smoke jobs (e.g. in PR 92). The test now reliably hits the error path and captures the expected message on all platforms. Also improves robustness of the npm wrapper smoke test added in #88. --------- Co-authored-by: Gnanam <gnanam@local> Co-authored-by: KRATOS <kratos@KRATOSs-Mac-mini.local>
Summary
The merged permission and verification event contracts added typed snapshots for config, providers, models, and sessions, but the persistent sandbox grant store (
internal/sandbox/grants.go) still exposes onlysandbox.Grantdirectly. The TUI render path, the headlesszero sandboxcommand, and PR/CI automation all need a typed snapshot so they do not hand-rollmap[string]anypayloads and do not accidentally leak grant reasons that contain API keys or tokens.This commit adds
SandboxGrantSnapshotalongside the existingProviderSnapshot,ModelSnapshot,SessionSnapshot, andSessionTreeSnapshotininternal/zerocommands. The snapshot mirrors the format of the existing typed contracts:ToolName,Decision,MaxAutonomy,ApprovedAtare copied verbatim from the underlying grant. They are non-secret metadata.Reasonis run through the standard redaction pipeline before it is copied, so any secret material the user typed when granting is masked before the snapshot leaves the runtime.Reasonbecomes an emptyReasonin the snapshot so JSON output omits the field.Two constructors are provided:
SandboxGrantSnapshotFromGrant(grant)for a single grant.SandboxGrantSnapshots(grants)for a slice. Output is sorted alphabetically byToolNameso consumers (TUI, headless, JSON output) see a deterministic ordering. An empty input returns a non-nil empty slice so JSON output is always[]and nevernull.A matched/unmatched helper is also added:
SandboxGrantMatchSnapshotFromLookup(toolName, lookup)pairs the typed snapshot with aMatchedboolean. When the lookup did not match, the returned snapshot hasMatched=falseand a nilGrantpointer, so consumers can render the absence without inspecting an error or a typed zero value.Tests
TestSandboxGrantSnapshotFromGrantRedactsReasonAndTrimsFields: verifies the secret in the reason is masked and the surrounding fields are trimmed.TestSandboxGrantSnapshotFromGrantEmptyReasonBecomesEmpty: verifies whitespace-only reasons normalize to the empty string.TestSandboxGrantSnapshotsSortsByToolNameAndReturnsEmptySliceForEmptyInput: verifies alphabetical sort and that nil input returns a non-nil empty slice whose JSON encoding is[].TestSandboxGrantMatchSnapshotFromLookupMatchedAndUnmatched: verifies the matched and unmatched code paths.TestSandboxGrantSnapshotJSONShapeIsStable: verifies every JSON key is present so downstream consumers can rely on the shape.Verification
go build ./...go vet ./...go test -count=1 -p 1 ./internal/zerocommands/... ./internal/sandbox/... ./internal/redaction/...DRI
Gnanam (runtime core owner per
docs/WORK_SPLIT_PRD.md§4). Connectszero sandboxcommand rendering, the TUI grants panel, and PR/CI automation consumers to the persistent grant store without leaking grant reasons.Out of scope
This PR only adds the typed contract and the constructors. The TUI render path, the
zero sandboxheadless command, and the PR/CI automation consumer are follow-up work in their respective owners' lanes.Summary by CodeRabbit
Tests
Chores