Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions IMPLEMENTATION_PLAN.md
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,9 @@ Done when:
can persist.
- [x] A prompt excludes candidates that existing session or persistent grants
already cover, while it preserves exact directory-scoped occurrences.
- [x] Reviewed-safe phrases grant no implicit authority to a headless, reminder,
or webhook run. Unattended candidates need explicit one-time or stored-grant
authority; approval-exempt shell side effects retain their bounded exception.
- [x] A one-time retry is bound to the exact prompted candidate set, including
each effective directory, across live, sub-agent, and redrive paths.
- [x] External paths, mismatched grants, dynamic syntax, and hard-deny rules
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,18 @@ own or inspect one-time approval state. The coordinator SHALL import actor
coverage, apply safe policy to still-uncovered candidates, validate the
invocation-owned one-time set exactly, and SHALL NOT rescan grants.

Reviewed-safe phrase coverage SHALL cover a candidate only when the run has
interactive approval capability. A run without that capability SHALL require
explicit one-time, session, or persistent authority for every candidate that
is not an approval-exempt side effect.

The actor result SHALL include typed persistent-store status. An absent store
file SHALL be ready with an empty snapshot. Expected corruption or migration
failure SHALL be unavailable. Completion SHALL allow a call fully covered by
one-time, session, or reviewed-safe authority without persistent state. If any
candidate remains uncovered and persistent state was unavailable, completion
SHALL return terminal `ApprovalStoreUnavailable` instead of a prompt.
one-time, session, approval-exempt side effects, or, for an interactive run,
reviewed-safe phrase coverage without persistent state. If any candidate
remains uncovered and persistent state was unavailable, completion SHALL
return terminal `ApprovalStoreUnavailable` instead of a prompt.

`ToolApprovalAttempt` SHALL remain owner of one-time invocation state.
`ToolApprovalActor` SHALL remain owner of session and persistent grants. The
Expand All @@ -49,11 +55,28 @@ a fact that no current type represents.
#### Scenario: Independent coverage survives unavailable persistence

- **GIVEN** the persistent store is unavailable
- **AND** interactive approval capability is available
- **AND** session and reviewed-safe coverage jointly cover every candidate
- **WHEN** completion evaluates the actor result
- **THEN** the call is allowed
- **AND** no persisted grant is assumed

#### Scenario: Reviewed-safe policy does not grant headless authority

- **GIVEN** interactive approval capability is unavailable
- **AND** a complete candidate is in the reviewed-safe catalog
- **WHEN** no one-time, session, or persistent grant covers that candidate
- **THEN** the candidate remains uncovered
- **AND** the caller follows the current unsupported-channel denial path

#### Scenario: Explicit grant covers a headless candidate

- **GIVEN** interactive approval capability is unavailable
- **AND** a session or persistent grant covers a complete candidate
- **WHEN** completion evaluates the call
- **THEN** the explicit grant covers that candidate
- **AND** reviewed-safe policy adds no authority

#### Scenario: Uncovered candidate fails closed when persistence is unavailable

- **GIVEN** the persistent store is unavailable
Expand Down
11 changes: 8 additions & 3 deletions openspec/changes/structure-shell-approval-policy/tasks.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,18 @@
covered and call-level invariants pass.
- [x] 3.3 Make internal exceptions, invalid enums, duplicate candidate IDs,
mismatched actor results, and impossible transitions terminal deny.
- [x] 3.4 Allow fully one-time/session/safe-covered calls when persistent state
is unavailable; deny with `ApprovalStoreUnavailable` instead of prompting
when any candidate still depends on that state.
- [x] 3.4 Allow calls covered by one-time or session authority and
approval-exempt side effects when persistent state is unavailable. Also allow
reviewed-safe phrase coverage for an interactive run. Deny with
`ApprovalStoreUnavailable` rather than open a prompt when any candidate still
depends on persistent state.
- [ ] 3.5 Let expected unresolved shell input offer only one-time approval and
deny; never create a reusable candidate.
- [ ] 3.6 Keep legacy token scans deny-only and prove they cannot authorize,
create persistence choices, or widen scope.
- [x] 3.7 Apply reviewed-safe phrase coverage only when interactive approval is
available. Prove that unattended calls need explicit one-time or stored-grant
authority while approval-exempt side effects keep their current behavior.

## 4. Typed grant phrases and persistence

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,11 @@ public Task Power_shell_approval_contract(string caseId)

private async Task AssertApprovalContract(string caseId)
{
var testCase = ShellApprovalCases.Get(caseId);
await AssertApprovalContract(ShellApprovalCases.Get(caseId));
}

private async Task AssertApprovalContract(ShellApprovalCase testCase)
{
await using var harness = await ShellApprovalHarness.CreateAsync(
testCase,
fixture.ActorSystem,
Expand All @@ -43,6 +47,43 @@ private async Task AssertApprovalContract(string caseId)
Assert.Equal(testCase.Expected.ApprovalMatches, observed.ApprovalMatches);
}

[Fact]
public Task Interactive_reviewed_safe_candidate_uses_reviewed_policy()
=> AssertApprovalContract(new ShellApprovalCase(
"interactive-reviewed-safe-allows",
new ShellApprovalInvocation("git status"),
Approvals.None,
ExpectedApproval.Allow(ToolAllowReason.SafeVerbInTrustedScope)));

[Fact]
public Task Noninteractive_reviewed_safe_candidate_stays_uncovered()
=> AssertApprovalContract(new ShellApprovalCase(
"noninteractive-reviewed-safe-requires-approval",
new ShellApprovalInvocation("git status", Interactive: false),
Approvals.None,
ExpectedApproval.Require(["git status"])));

[Fact]
public Task Noninteractive_candidate_can_use_an_explicit_persistent_grant()
=> AssertApprovalContract(new ShellApprovalCase(
"noninteractive-reviewed-safe-with-grant-allows",
new ShellApprovalInvocation("git status", Interactive: false),
Approvals.PersistentAnywhere("git status"),
ExpectedApproval.Allow(
ToolAllowReason.StoredApproval,
1,
"persistent:git status")));

[Fact]
public Task Noninteractive_safe_candidate_does_not_fill_a_partial_grant_gap()
=> AssertApprovalContract(new ShellApprovalCase(
"noninteractive-partial-grant-keeps-safe-candidate-uncovered",
new ShellApprovalInvocation("git push && git status", Interactive: false),
Approvals.PersistentAnywhere("git push"),
ExpectedApproval.Require(
["git status"],
approvalMatches: ["persistent:git push"])));

[SlopwatchSuppress("SW001", "This regression requires POSIX symlink and Bash authorization behavior.")]
[Fact(SkipUnless = nameof(IsPosix), Skip = "The symlink retry regression defines Bash authorization behavior.")]
public async Task One_time_retry_rechecks_candidates_that_become_unsafe()
Expand Down
5 changes: 4 additions & 1 deletion src/Netclaw.Actors/Tools/ShellPolicyCoordinator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,11 @@ approvalService is not null
ShellScopeRelation.None);
}

var canUseReviewedSafePolicy =
projection.RunScope.InteractiveApproval is InteractiveApprovalCapability.Available;
foreach (var candidate in grantCandidates.Where(candidate =>
coverage.UncoveredIds.Contains(candidate.Id)))
canUseReviewedSafePolicy
&& coverage.UncoveredIds.Contains(candidate.Id)))
{
if (policy.IsReviewedSafeCandidate(
candidate.Candidate,
Expand Down
Loading