Skip to content

Enforce sensitivity ceilings at tool and response boundaries - #657

Merged
imran-siddique merged 1 commit into
mainfrom
agent/tool-sink-ceilings
Sep 18, 2026
Merged

imran-siddique merged 1 commit into
mainfrom
agent/tool-sink-ceilings

Conversation

@imran-siddique

Copy link
Copy Markdown
Member

What

Add optional operator-owned sensitivity ceilings for tool destinations and caller responses. Once a session handles confidential data, a clean-looking summary cannot be sent to a public-only tool by declaring it public. The actual gateway path denies before discovery, checks again after discovery and before dispatch, then enforces the response ceiling after inspection. The checks remain hard denials in advisory and silent modes.

Why

Cedar policy and scanning did not provide a separate destination clearance limit. The new policy combines accumulated session, catalog and declared classifications, denies unlisted tools and unknown labels, and preserves the call-entry floor across an in-flight session reset. Configuration is immutable after proxy creation. Omitting the policy preserves compatibility. This follows #656.

Security impact

Configured gateways suppress captured child-process stderr content. Selected upstream and policy exception logs retain codes/types instead of potentially private messages.

This is conservative label enforcement, not semantic information-flow tracking or automatic declassification. It requires truthful classification and trusted configuration. Direct agent sockets/files, other model calls, remote-tool confidentiality and other logging paths remain outside this gateway control. Audit hashes and metadata remain potentially sensitive. Response denial cannot undo an executed tool operation.

Test plan

  • Unit, conformance and integration: 1,902 passed, 8 skipped; 88.87% coverage on Windows/Python 3.12
  • Ruff over source/tests, runtime mypy (59 modules), Bandit and strict MkDocs build passed
  • 31 new tests exercise real proxy admission/response paths, classification ratchets, modes, resets, configuration errors and logging
  • Disabling sink checks causes 12 regression failures; ignoring stderr suppression causes 2; restoring upstream plaintext logging causes 1; removing the post-discovery check causes 1
  • Merged-main tree matches the tested base exactly

No new hardware validation is claimed. CI will exercise its platform matrix separately.

DCO sign-off

  • Contribution is submitted with DCO sign-off.

Signed-off-by: Imran Siddique <imran.siddique@opaque.co>

@kingztech2019 kingztech2019 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed against the merged tree at f98c6c9 rather than the diff, since the two things I wanted to check (exception attribution at the second checkpoint, and whether the two validate() calls use the same rank map) are both invisible in a patch. Both turned out fine, and one hypothesis I had was wrong in a way worth recording.

Verified

Checkpoint ordering is exactly as documented, traced through the failure_stage sequence: catalog_lookup (1454), sink_admission (1500), upstream_drift_check (1536, where discovery awaits), policy_evaluation (1593, the recheck), session_update (1901), egress_policy (1917). Admission does precede discovery, and the response ceiling does run after the session update, so both claims in the spec hold.

Response denial suppresses the payload (response=None), records egress_denied, and hardcodes would_have_denied=False, so it stays a hard deny in advisory and silent modes as stated.

Attribution at the Cedar checkpoint is correct. PolicyDeny.__init__ always sets advice and aarm_decision, so the bare PolicyDeny("sink_policy:...") raised from require() cannot trip an AttributeError inside the handler that reads them. decision_for_deny({}) returns DENY, so the record is policy_decision="deny" with policy_rule_matched="sink_policy:tool_denied", not mislabeled as a Cedar rule.

The double validate() is not redundant, so please keep both. effective_sensitivity_order(config.sensitivity.vocabulary) and the inlined {**sensitivity_vocabulary, **SENSITIVITY_ORDER} at config.py:536 are the same map, but the tests construct Config(...) directly and bypass load_config, which makes the proxy-init call the only validation on that path.

The hypothesis that failed, and why it is worth stating

I went looking for a way to make the recorded session sensitivity read lower than the truth, since the entire guarantee rests on that. The candidate was the reset race: update_from_inspection returns False on a reset_count mismatch before applying any tags, and the proxy only logs SESSION_RESET_RACE and continues to the egress check. So a response's sensitivity can be discarded while the call proceeds.

That is not exploitable, because response_sensitivity is [entry.sensitivity_level, declared_data_class] (proxy.py:1889) and both values are passed into the egress labels tuple as raw inputs. Worth saying plainly: passing all four raw sources (call-entry floor, live session value, catalog level, declared class) instead of trusting the single accumulated number is what makes this gate robust. Any individual failure of the accumulation path still trips it. That property is the strongest part of the design and it is not visible in the diff, so it is worth a comment in require() or the spec so a later refactor does not "simplify" the tuple down to self._session.max_sensitivity and quietly remove the redundancy.

One substantive finding: two of the three log reductions are unconditional, but documented as opt-in

The stderr change is gated on the policy being configured (proxy.py:708, log_stderr=self._sink_policy is None). The other two are not:

# proxy.py:1729, unconditional
logger.warning("Upstream call failed: tool=%s code=%s", tool_name, exc.code)

# proxy.py:1660, unconditional, exc_info=True dropped
logger.error("CEDAR_FAULT: tool=%s exception_type=%s", tool_name, type(exc).__name__)

Every documentation surface frames the logging boundary as a consequence of enabling the feature. docs/spec/sink-policy.md opens that paragraph with "When this policy is configured". LIMITATIONS.md says "With this option". The CHANGELOG joins both clauses under one subject: "Strict sink policy suppresses captured stdio stderr content; upstream error logs no longer echo tool messages." A deployment that never sets sink_policy still gets both reductions.

The exc_info=True removal is the part I would push back on. That handler is the malformed-policy fault path, and its own comment says the entry exists "so the incident is traceable". Dropping the traceback makes a 500-causing Cedar fault materially harder to diagnose for every existing operator, including those who never opt in, and the CHANGELOG does not mention that change at all. Since code is a class constant on CMCPError, the upstream log now distinguishes only two possible values and carries no detail, not even exc.detail.

Any of these resolves it:

  1. Gate both like stderr, keeping full detail when self._sink_policy is None.
  2. Keep the traceback while dropping only the message: exc_info=(type(exc), None, exc.__traceback__). A Cedar backend traceback is gateway-internal control flow; str(exc) is the part that could embed input.
  3. If the reductions are meant to be unconditional, state that plainly in the CHANGELOG and the spec, and add the Cedar traceback change to the CHANGELOG.

Two nits

stage_results keys diverge for the same logical gate: {"sink_policy": "deny"} at the pre-discovery checkpoint, {"policy": denied_as} at the Cedar checkpoint. An operator filtering on stage_results["sink_policy"] to count sink denials therefore misses exactly the race denials the second checkpoint exists to catch. policy_rule_matched is consistent across both, so this is observability only, not correctness.

__post_init__ assigns a MappingProxyType to a field on a frozen=True dataclass, which generates __hash__. hash(SinkPolicy(...)) raises TypeError: unhashable type: 'dict' (mappingproxy delegates to the underlying dict). Equality is unaffected. Latent today since nothing hashes it, but this project cares about configuration digests and the spec already notes the policy is not bound into an attestation claim, so if that ever lands this will surface. Storing tuple(sorted(...)) alongside, or setting eq=False, avoids it.

Verdict

Approving. The enforcement path is correct, and the fail-closed behavior is right on unlisted tools, unknown labels, and empty label sets. The log gating question above is worth settling before merge, but it is separable from the feature itself.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants