Approved mechanistic policy chunk rejects itself when a denial resubmits the same endpoint (dedup upsert id aliasing in SubmitPolicyAnalysis)

Agent Diagnostic

Investigation started from NVIDIA/NemoClaw#6329 (approved network rule shown as rejected mid-session with no human action). Traced through this repo's source rather than a live cluster:

Read crates/openshell-server/src/grpc/policy.rs (SubmitPolicyAnalysis flow: put_draft_chunk -> supersede_other_pending_chunks_for_endpoint -> self_reject_mechanistic_if_already_covered -> auto_approve_chunk), crates/openshell-server/src/persistence/sqlite.rs (put_draft_chunk upsert), crates/openshell-server/src/policy_store.rs (observation_dedup_key), crates/openshell-sandbox/src/lib.rs (flush_proposals_to_gateway), crates/openshell-sandbox/src/mechanistic_mapper.rs (generate_proposals, generate_rule_name).
Enumerated every production writer of chunk status "rejected": the supersede pass (scans pending only), the human RejectDraftChunk handler (also un-merges via remove_chunk_from_policy and emits a removed audit log), and self_reject_mechanistic_if_already_covered. Only the last can flip an approved row without human action.
Verified the defective code is identical on main (abe42fb5) and on the v0.0.72 tag (the reporter's deployed version), and that the self-reject path first shipped in v0.0.53 (e98ea3ee, feat(policy): add agentic approval loop #1528).
Not reproduced on a live gateway; this is a source-level trace. The reporter-side evidence (screenshots + OCSF logs in nemoclaw openshell container updating network policy during agent turns NemoClaw#6329) matches every step.

Description

An approved mechanistic draft chunk can be flipped to rejected by the gateway itself, with no human action, leaving the governance ledger inconsistent with the enforced policy.

Chain (all in SubmitPolicyAnalysis):

A denial inside the sandbox triggers the denial-analysis flush, which resubmits a mechanistic proposal for the denied endpoint (flush_proposals_to_gateway -> generate_proposals; denials are grouped by (host, port, binary) with no stage filter, so L7 method denials resubmit too).
put_draft_chunk upserts mechanistic chunks on dedup_key = "{host}|{port}|{binary}". On conflict it only bumps hit_count, and RETURNING "id" hands back the existing row's id. If the endpoint's chunk was already approved, effective_id now names the approved chunk.
self_reject_mechanistic_if_already_covered(state, sandbox_id, &effective_id, host, port, binary) scans approved chunks for one covering (host, port, binary):
```
let covered_by = approved
    .iter()
    .find(|c| c.host == host && c.port == port && c.binary == binary);
```
It neither excludes new_chunk_id from the scan nor checks that the chunk it is about to reject is still pending. The approved chunk "covers itself" and gets update_draft_chunk_status(new_chunk_id, "rejected", ...) — rejected with reason "already covered by approved chunk ".

Consequences:

Unlike the human reject path (handle_reject_draft_chunk_inner), this path never calls remove_chunk_from_policy. The merged policy still contains the rule, so enforcement and ledger now disagree: TUI/audit show [rejected] review required while the rule is still active. An operator reading the TUI believes access was revoked when it is not.
The chunk is stuck: subsequent dedup resubmissions land on a rejected row (hit_count keeps climbing), and auto_approve_chunk only approves from pending, so the endpoint can never be re-approved through this path.
Expected asymmetry per the function's own doc comment ("only the mechanistic side is asymmetric... incoming mechanistic drafts") is violated: a previously granted approval is silently revoked in the ledger.

Field evidence (NVIDIA/NemoClaw#6329): rule allow_172_17_0_1_8090 — the exact generate_rule_name mechanistic pattern — went from [approved] 50% auto-approved 17x to [rejected] 50% review required 18x between two screenshots hours apart; the +1 hit count is the dedup upsert's ON CONFLICT bump, and the logs show an L7 DELETE /v1/mcp denial followed by Flushed denial analysis to gateway right in that window.

Reproduction Steps

Unit-level (drives the exact production functions):

Submit a mechanistic policy analysis for endpoint (hostA, 8090, python3.13) with auto-approve enabled (or approve the resulting chunk) so the chunk reaches approved.
Submit a second mechanistic analysis for the same (host, port, binary) (as any new denial for that endpoint would).
Observe: put_draft_chunk dedups into the approved row and returns its id; self_reject_mechanistic_if_already_covered flips it to rejected; hit_count incremented; no remove_chunk_from_policy call.

Expected: the approved chunk stays approved (hit count may increment); only genuinely new pending drafts are ever auto-rejected.

Live sketch: sandbox with an approved mechanistic rule for an endpoint whose L7 policy still denies some method (e.g. DELETE); trigger that method from inside the sandbox; wait for the denial flush; watch the rule flip in the TUI.

Suggested fix

let covered_by = approved
    .iter()
    .find(|c| c.id != new_chunk_id && c.host == host && c.port == port && c.binary == binary);

plus, defensively, only reject when the effective chunk's current status is pending (the dedup upsert can return a non-pending row; nothing downstream should treat it as a fresh draft). I am happy to submit this as a PR with the regression test above (first-time external contributor — I understand vouching applies).

Environment

Observed in the field via nemoclaw openshell container updating network policy during agent turns NemoClaw#6329: Ubuntu, NemoClaw 0.0.71, OpenShell 0.0.71/0.0.72, Hermes agent sandbox.
Code verified identical on tag v0.0.72 and main (abe42fb5); introduced in v0.0.53 (e98ea3ee, feat(policy): add agentic approval loop #1528).

Logs

2026-07-06T13:16:38.626Z OCSF HTTP:DELETE [MED] DENIED /usr/bin/python3.13(392063) -> DELETE http://172.17.0.1:8090/v1/mcp [policy:spiceai engine:l7] [reason:FORWARD_L7 deny DELETE 172.17.0.1:8090/v1/mcp reason=DELETE /v1/mcp not permitted by policy]
2026-07-06T13:16:44.916Z INFO openshell_sandbox: Flushed activity summary to gateway
2026-07-06T13:16:44.917Z INFO openshell_sandbox: Flushed denial analysis to gateway

TUI before/after (screenshots in NVIDIA/NemoClaw#6329): allow_172_17_0_1_8090 [approved] 50% auto-approved 17x -> allow_172_17_0_1_8090 [rejected] 50% review required 18x.

📋 triage-agent

Classification: bug-confirmed

Summary: An approved mechanistic policy chunk can silently reject itself when a denial resubmits the same endpoint. The self_reject_mechanistic_if_already_covered function does not exclude new_chunk_id from its scan, so an approved chunk "covers itself" and gets flipped to rejected. This leaves the governance ledger inconsistent with the enforced policy.

Investigation:

Confirmed in source at crates/openshell-server/src/grpc/policy.rs:769-818. The self_reject_mechanistic_if_already_covered function scans all approved chunks for the same (host, port, binary) match (line 799-801) but does not filter out new_chunk_id:
```
let covered_by = approved
    .iter()
    .find(|c| c.host == host && c.port == port && c.binary == binary);
```
When put_draft_chunk dedup-upserts into an already-approved row and returns its ID, the approved chunk matches itself and gets rejected.
The rejection path (line 810-817) calls update_draft_chunk_status but, unlike the human RejectDraftChunk handler, does NOT call remove_chunk_from_policy. This creates a ledger/enforcement mismatch: the TUI shows "rejected" while the merged policy still contains the rule.
The reporter's suggested fix (adding c.id != new_chunk_id to the filter, plus a pending-status guard) is correct and minimal.
Field evidence from nemoclaw openshell container updating network policy during agent turns NemoClaw#6329 matches the traced code path exactly.
Bug introduced in v0.0.53 (feat(policy): add agentic approval loop #1528), still present on main.

Recommendation: Confirmed bug with security/governance implications (ledger says access revoked but rule remains active). The reporter has offered to submit a PR with a regression test. Priority fix recommended.

Following up on the triage confirmation: the offer to send this as a PR still stands — the scan fix above (exclude new_chunk_id from the covering-chunk lookup), the defensive only-reject-when-pending check, and the regression test described in the issue body (approve a mechanistic chunk, resubmit the same endpoint mechanistically, assert it stays approved with hit_count incremented).

The only blocker on my side is vouching: PRs from unvouched contributors are auto-closed, and my vouch request (#2176, opened July 8) is still awaiting a response. I'll submit as soon as that clears — or if a maintainer prefers to take the fix directly in the meantime, everything needed is in the issue body.

@Hokonoken

@Hokonoken sorry, my apologise for my comment, I selected the wrong issue for a triage workflow targetting a different issue (and haven noticed the creation of that comment). I haven't reviewed the comment above (what I unconditionally do always when I'm not doing mistakes like this). So, while the comment might be valid, its not something I can verify now (haven't dived into the issue), so please don't take this as normative. (yep, I'm still fully responsible for that comment and will try to dig deeper, but need to catch up on the original content).

No worries at all, and thanks for the transparency. To keep expectations accurate on my side too: the analysis in the issue body is a code trace, not a live reproduction — the regression test described there (approve a mechanistic chunk, resubmit the same endpoint mechanistically, assert it stays approved with hit_count incremented) is what would confirm the behavior. The PR offer stands whenever the vouching question (#2176) is settled, and there's no urgency from my side — the field report this came from (NVIDIA/NemoClaw#6329) has a workaround in place.

📋 triage-agent

Triage Assessment

Classification: bug-confirmed

Summary

This is a deterministic policy-governance defect, not user error and not a duplicate. A repeated mechanistic observation can deduplicate onto an already-approved chunk, after which the self-reject path lets that chunk match itself and changes its ledger status to rejected while the merged rule remains enforced.

Investigation

put_draft_chunk returns the existing row ID when a mechanistic (host, port, binary) key conflicts. self_reject_mechanistic_if_already_covered then includes that same ID in its approved-chunk search and calls the unconditional status-update path, producing an enforcement/ledger mismatch and leaving subsequent resubmissions stuck on the rejected row.

Open PR #2242 contains the correct direct protections for the reported aliasing path and a focused handler-level regression test. Branch checks are green. Review should additionally ensure that pending to rejected is an atomic conditional transition in both persistence backends; the current PR reads pending and later writes rejected unconditionally, leaving a concurrency window in which an intervening approval could recreate the same mismatch.

Recommendation

Route to the policy and gateway owners and track the linked fix in #2242. The issue can close when that PR (or an equivalent fix with the atomic status transition) merges.

fix(policy): keep approved chunk when a mechanistic denial resubmits …

@laitingsheng

Confirmed — #2242 (merged) fixes this at the exact point traced above, self_reject_mechanistic_if_already_covered:

it now self-rejects only a genuinely fresh, still-pending submission (a get_draft_chunk(new_chunk_id) status guard), and
it excludes new_chunk_id from the covering-approved-chunk search — so a dedup upsert that aliases the incoming id onto an already-approved row can no longer make that row reject itself.

Those are the two defects reported here (no self-exclusion + no pending-status guard).

It also closes the concurrency window raised in the triage: the final status write is now an atomic compare-and-swap — conditionally_reject_draft_chunk (returns whether the row actually changed), implemented in both the SQLite and Postgres backends with regression tests — instead of a read-then-unconditional-write.

Verified by reading the merged diff, not a live cluster run. Good to close from my side — thanks @laitingsheng.

Chore/resync 20260820 (#4)

Navigation Menu

Repository navigation

Approved mechanistic policy chunk rejects itself when a denial resubmits the same endpoint (dedup upsert id aliasing in SubmitPolicyAnalysis) #2165

Agent Diagnostic

Description

Reproduction Steps

Suggested fix

Environment

Logs

Triage Assessment

Summary

Investigation

Recommendation

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Approved mechanistic policy chunk rejects itself when a denial resubmits the same endpoint (dedup upsert id aliasing in SubmitPolicyAnalysis) #2165

Description

Agent Diagnostic

Description

Reproduction Steps

Suggested fix

Environment

Logs

Activity

rhuss commented on Jul 12, 2026

Hokonoken commented on Jul 13, 2026

rhuss commented on Jul 13, 2026

Hokonoken commented on Jul 13, 2026

TaylorMutch commented on Jul 13, 2026

Triage Assessment

Summary

Investigation

Recommendation

Hokonoken commented on Jul 14, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions