Skip to content

TaintResult cannot name a pair: exhausted drops the selector's scope, and the ledger's pair association survives only as prose #403

Description

@rahlk

Describe the bug

TaintResult has no way to name a (source, sink) pair, and two of its fields need one.

exhausted drops the selector's scope. A caller passes (name, within) selectors, but cldk/analysis/commons/graphs.py:723 keeps only the name half when it records a pair:

for (source, _), a in zip(sources, srcs):
    for (sink, _), b in zip(sinks, dsts):
        pairs.setdefault((a.ref, b.ref), (source, sink, a))

The _ is the within. Pairs are keyed internally by (a.ref, b.ref), so two selectors that share a variable name in different callables are correctly kept apart during the walk and then reported identically. Measured against the shipped taint_verdict:

sources=[("x", "A.f"), ("x", "B.g")]
sinks=[("y", "C.h")]
# exhausted: [('x', 'y'), ('x', 'y')]   -- 1 distinct spelling for 2 distinct pairs

Nothing is falsely certified — both pairs genuinely were searched to exhaustion — so this is not a soundness bug. It is an auditability one, which matters because exhausted is the field a triage caller acts on to close an alert. The caller cannot tell which requested pair an entry refers to, and set(r.exhausted) or a dict keyed on it silently collapses distinct verdicts into one.

The ledger's pair association exists only in prose. The committed spec (docs/design/specs/2026-09-09-leg-4-taint.md, §5) specifies unresolved as "the frontier ledger, tagged with the pair each affects". The shipped implementation cannot do that: Diagnostic has no structured field for a subject, so the pair is written into message as human-readable text, and TaintResult's own docstring is explicit that parsing it back out is forbidden — that would resurrect the derivation the stored exhausted exists to avoid. The association is tracked internally, correctly, and then discarded at the boundary. A caller who wants the spec's promise has to re-derive it, which is the error class the field was designed to remove.

Both symptoms are the same missing thing: a pair is not addressable in the returned types. The spec specifies exhausted: list[Tuple[str, str]] but never says how a pair is spelled, so this is under-specification rather than a deviation.

Found while reviewing the leg 4b surface (#382, PR #393). Not a blocker for that PR: no verdict is wrong, and the collision needs a shape no current test uses. Related: the design record for this surface is docs/design/specs/2026-09-09-leg-4-taint.md, still unmerged on PR #383.

To Reproduce

Not stated in the original issue.

Expected behavior

  • A test constructs two requested pairs whose sources share a variable name in different callables, both refuted, and asserts the two exhausted entries are distinguishable — an exact expected list, not a length check. It fails on current main and passes after.
  • Every Diagnostic in unresolved carries its pair in a structured field, asserted by reading that field, never by matching message text.
  • exhausted's three conditions are unchanged, demonstrated by the existing taint suites staying green without edits to their verdict assertions.
  • TaintResult.model_validate(r.model_dump()) still round-trips, asserted on a result with a non-empty exhausted and a non-empty unresolved.
  • docs/agent-api-reference.md, the TaintResult docstring, and the leg-4 spec's §5 all state the same pair spelling.
  • If the element type changed, CHANGELOG.md records it under breaking changes with the migration line.

Additional context

Scope boundary

Makes a pair addressable in TaintResult, and only that. It does not:

  • change which pairs land in exhausted — the three conditions at cldk/analysis/commons/graphs.py:753 are correct and stay as they are;
  • change the traversal, the cut predicate, or any verdict;
  • add a verdict enum. The leg-4 spec rejected one (T-series decisions) and this does not reopen it;
  • widen Diagnostic for any consumer other than the taint ledger, if the chosen fix touches Diagnostic at all.

Both symptoms are fixed together because they are one decision. Fixing only exhausted leaves the spec's unresolved promise unmet.

Goals

  • Decide the pair's wire form. Two candidates, and the choice is a published-contract decision rather than a mechanical one: (a) exhausted: list[tuple[tuple[str, str], tuple[str, str]]] — the caller's selectors verbatim, which is self-describing but nests; (b) an additive subject field on Diagnostic plus the same pair shape on exhausted, which serves both symptoms with one concept.
  • Implement it in taint_verdict only — the assembly already lives once in cldk/analysis/commons/graphs.py, so this is one edit, not three.
  • Populate the pair association on every unresolved diagnostic, replacing the prose-only convention for degenerate_pair and unresolved_dispatch.
  • Update docs/agent-api-reference.md (the exhausted rules at :782-789 and the diagnostic table at :966) and the TaintResult docstring, which currently documents the prose convention as deliberate.
  • Amend the leg-4 spec's §5 to state the pair's spelling, since it is the omission that let this through.

Caveats and known risks

  • This widens a published model contract. TaintResult and Diagnostic are documented in docs/agent-api-reference.md, and exhausted's element type would change shape under option (a). Any caller unpacking for src, sink in r.exhausted breaks. That argues for doing it before 2.0.0 final rather than after, and for saying so in the changelog under breaking changes.
  • Option (b) touches Diagnostic, which every accessor in the SDK returns. An optional field is additive at the type level, but it is a model every language's backend constructs, so the blast radius is wider than taint.
  • TaintResult round-trips through model_dump/model_validate in tests/analysis/commons/test_taint_semantics.py:191; a nested tuple shape needs that test to keep passing, and tuples do not survive JSON as tuples.
  • The collision requires same-named variables in different callables across a multi-source call, so no existing test exercises it and none fails today. A test that fails before the fix has to be written first, or the fix is unwitnessed.
  • Under option (a) the exhausted entry no longer matches the spelling degenerate_pair's message uses, so the two must be reconciled in the same change or the ledger and the verdict field will describe the same pair two ways.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions