Skip to content

Allow-once is consumed by an unrelated subsequent text change (agent-os-vscode policyEngine) #2188

@finnoybu

Description

@finnoybu

Problem

PolicyEngine.allowOnce / analyzeCode currently records an allow-once exemption as a rule-name set:

// Skip if allowed once
if (this.allowedOnce.has(rule.name)) {
    this.allowedOnce.delete(rule.name);  // Remove after one use
    continue;
}

The exemption is keyed only on rule name with no association to the text that originally triggered the block. That means a subsequent text change can silently consume the exemption even when the change is unrelated to the rule the user wanted to allow:

  1. Drift consumption. User triggers rule unsafe_eval, clicks "Allow once". Then deletes that eval(...) line entirely and types a new function. Next analyzeCode call finds no unsafe_eval match → no allow-once is needed → but the exemption is still in the set. Later they type a new eval(...) for a different reason; the still-pending exemption silently allows it. Surprise.
  2. Wrong-occurrence consumption. A file contains two eval(...) sites. User allows-once at site A, expecting that one analysis run to ignore site A. The next analysis pass sees both sites — rule.pattern.test(code) returns true for the whole document — and the single allow-once is consumed for whichever site the iteration order happens to land on. The user has no UI signal which they actually allowed.
  3. Cross-document leakage. analyzeCode(code, language) doesn't bind allow-once to the document or workspace. An exemption set in document A is consumed by the next call from document B for the same rule, again silently.

All three modes produce the same end-user observation: "I clicked Allow once, then later the same kind of code I expected to be blocked wasn't."

Why this isn't an obvious fix

There are several reasonable shapes the fix could take and the choice is a product decision:

  • Hash-bound exemption. Allow-once becomes (rule, sha256(matched-substring)) or (rule, sha256(entire-document)). Consumed only when the same content is re-analyzed. Strictest semantics, but the user has to re-click on any edit.
  • Document-bound TTL. (rule, documentUri, expiresAt) with e.g. 5-second / next-analysis-call lifetime. Tolerates small edits while the user is mid-typing, expires before unrelated edits arrive.
  • Active-block-bound. Allow-once only matters when an analyzeCode call would otherwise block. If the next call wouldn't block on that rule, the exemption stays armed; if it would, it's consumed. Closest to user mental model but requires reshaping the loop in analyzeCode.
  • Position-bound. Track the offset range that triggered the block; consume only when the same rule fires inside that range on the next analyze. Requires plumbing offsets through the pattern test.

The current behaviour is also a regression risk: changing semantics here will surprise users who have built workflows on top of the existing "next analysis only" interpretation. The right pick probably depends on telemetry the project owns and how the command palette wiring lays out "Allow once" UX.

Repro (minimal)

const engine = new PolicyEngine();
await engine.analyzeCode('eval(payload)', 'typescript');     // blocked
engine.allowOnce('unsafe_eval');                              // user clicks Allow once
await engine.analyzeCode('const x = 1;', 'typescript');       // no eval → no block, exemption silently consumed
await engine.analyzeCode('eval(other_payload)', 'typescript');// expected: blocked. Actual: also blocked, but
                                                              // had it landed before the second call,
                                                              // it would have been allowed.

A more dramatic variant: between calls 2 and 3, swap to a different document. The exemption is now armed across document boundaries.

Scope note

This is a behaviour / UX bug, not a security regression — the surrounding policy engine is advisory inside the editor and any false-allow caused by the wrong consumption is bounded by what the user explicitly clicked "Allow once" on at least one matching occurrence of. But the silent consumption is confusing enough that it's worth a deliberate fix.

Happy to follow up with a PR once a direction is chosen — I'd lean toward active-block-bound as the smallest behavioural change that fixes all three modes, but the project owners are better placed to make the call.


Surfaced during independent audit conducted by @finnoybu (Ken Tannenbaum, AEGIS Initiative); [LOW, TypeScript].

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions