Skip to content

feat(security): handle grant constraints, memory policy, redaction boundary tests#79

Merged
dgenio merged 2 commits into
mainfrom
claude/triage-issues-F9tbR
May 21, 2026
Merged

feat(security): handle grant constraints, memory policy, redaction boundary tests#79
dgenio merged 2 commits into
mainfrom
claude/triage-issues-F9tbR

Conversation

@dgenio
Copy link
Copy Markdown
Owner

@dgenio dgenio commented May 20, 2026

Summary

Implements the security-boundary group identified during triage: #76
(handle grant-constraint enforcement), #75 (memory action policy), and
#74 (redaction boundary tests). One PR because all three changes touch
the same seam ÔÇö what data is allowed to flow from driver/store outward, and
how policy + trace + firewall enforce it.

This PR also closed two related issues during triage: #73 (decision
trace format) and #77 (stable denial reason codes) ÔÇö both implemented
by PR #78 but the auto-close didn't fire. Each has a closing comment
explaining the evidence.

What changed

Handle grant-constraint enforcement (#76)

  • Handle now carries principal_id and constraints from the original
    grant, persisted at creation time by HandleStore.store.
  • HandleStore.expand rechecks against the requested expand query:
    max_rows, allowed_fields, scope filter, and principal binding.
    Violations raise HandleConstraintViolation (new exception, exported
    from agent_kernel) with stable reason_code.
  • An unscoped expand applies allowed_fields as the default projection
    and AND-merges scope into the filter, so a caller can't bypass either
    constraint by omitting it.
  • Kernel.expand gained an optional principal: Principal argument that
    is forwarded for the principal-binding check.

Memory action policy (#75)

  • New SensitivityTag.MEMORY.
  • DefaultPolicyEngine.evaluate adds a MEMORY branch after existing
    sensitivity checks (per the "rule placement matters" trap in
    docs/agent-context/invariants.md):
    • memory.read with scope.memory_scope == "sensitive" requires
      memory_reader_sensitive or admin.
    • memory.write / DESTRUCTIVE memory always requires memory_writer
      or admin.
    • Project-scoped reads are allowed by default.
  • explain() lists the same conditions with stable reason_codes.
  • Kernel-side redaction in ActionTrace.args: for capabilities whose ID
    starts with memory., payload-like keys (payload, content, value,
    memory, text, body) are replaced with "[REDACTED]". Keys are
    preserved so audit can still confirm the action.

Redaction boundary tests (#74)

  • New tests/test_firewall_boundary.py with 8 negative-assertion tests:
    • Synthetic secret/PII patterns never appear in summary/table frames.
    • allowed_fields strips disallowed columns AND their key names.
    • handle_only frame carries only a Handle reference (no rows, no raw).
    • Non-admin asking for raw is silently downgraded to summary;
      raw_data stays None.
    • Admin in raw mode receives raw data but inline secret patterns
      (Bearer tokens, API keys) are still scrubbed by redact().
    • ActionTrace.args for memory capabilities redacts payload-like keys.
    • The redaction is scoped to memory.* ÔÇö non-memory capabilities are
      untouched.

New stable denial codes (added to agent_kernel.policy_reasons):

  • HANDLE_CONSTRAINT_VIOLATION (handle_constraint_violation)
  • HANDLE_PRINCIPAL_MISMATCH (handle_principal_mismatch)
  • MEMORY_WRITE_REQUIRES_WRITER (memory_write_requires_writer)
  • MEMORY_SENSITIVE_READ_DENIED (memory_sensitive_read_denied)

Docs

  • docs/security.md: new "Handle expansion boundary" and "Memory actions"
    sections; expanded threat-model table; expanded confused-deputy section
    to cover handles.
  • docs/architecture.md: MEMORY entry in DefaultPolicyEngine rule list;
    four new rows in the reason-code table.
  • CHANGELOG.md: [Unreleased] documents the three changes.

Why

From triage: handle expansion was the load-bearing seam where over-broad
data could escape the original grant; memory writes persist into future
sessions so they need an explicit policy layer; and the redaction
boundary lacked focused negative-assertion tests pinning the contract.
The three issues share the same modules (policy.py, handles.py,
kernel.py, models.py, the firewall) so doing them together avoided
three separate rounds of refactoring the same files. Per Mode B and
explicit owner permission, no backwards-compatibility shims were added
to the Handle dataclass.

How verified

  • ruff format --check src/ tests/ examples/ ÔÇö 49 files already formatted.
  • ruff check src/ tests/ examples/ ÔÇö All checks passed.
  • python -m mypy src/ ÔÇö Success: no issues found in 30 source files.
  • python -m pytest -q --cov=agent_kernel ÔÇö 477 passed (was 450; +27
    new tests). Coverage 96%.
  • examples/basic_cli.py, examples/billing_demo.py,
    examples/http_driver_demo.py ÔÇö all run clean.

Tradeoffs / risks

  • No backwards compat on Handle (per Mode B): the dataclass gained
    two new fields with safe defaults (empty principal_id, empty
    constraints). Direct Handle(...) constructors that relied on
    positional-only args would break, but every existing test passes
    unchanged because all callers use keyword args.
  • MEMORY rule placement: explicitly placed after PII/PCI/SECRETS
    checks to honour the documented invariant ordering. New SensitivityTag
    enum values need a matching policy rule ÔÇö that rule was added in the
    same change.
  • Redaction is scoped to memory.* by capability-ID prefix, not by
    sensitivity tag, because the input-side redaction needs to apply
    regardless of whether the caller set the sensitivity tag correctly.
    Non-memory caps are unchanged.
  • Out of scope: did not decompose policy_dsl.py (issue [policy/kernel] Tech debt: decompose policy_dsl.py and broaden dry-run driver test coverage #68 D ÔÇö file
    is now 661 lines, even bigger). Did not add an OTel exporter, marketplace,
    or memory backend. Each can land separately.

Scope notes (Mode B)

Three issues, one PR. Adjacent issues left as separate follow-ups: #68
(tech debt + dry-run driver coverage), #38 (OTel), #45 (namespaces),
#46 (tutorial), #47 (streaming), #49/#51/#52 (marketplace), #71 (boundary
docs).

Closes

Closes #74, closes #75, closes #76.

(#73 and #77 already closed manually during triage ÔÇö implemented by PR
#78 but auto-close didn't fire.)

…edaction boundary tests

Implements the recommended security-boundary group (#74, #75, #76):

#76 — HandleStore.expand now enforces the grant constraints persisted on
the Handle at creation time. Requests that exceed the grant's max_rows,
ask for fields outside allowed_fields, or break the scope filter raise
HandleConstraintViolation with a stable reason_code. Handles are also
principal-scoped — expand by another principal raises with
HANDLE_PRINCIPAL_MISMATCH. Handle IDs are no longer bearer credentials.

#75 — SensitivityTag.MEMORY plus matching policy rules in
DefaultPolicyEngine. Sensitive-scoped memory reads require an explicit
role, durable writes always require memory_writer (or admin). The kernel
also redacts payload-like keys from ActionTrace.args for any capability
whose ID starts with "memory." so durable content never enters the
audit log.

#74 — New tests/test_firewall_boundary.py pins the raw -> Frame / Handle
/ ActionTrace seam with synthetic-secret negative assertions across
every response mode, plus the memory-trace redaction path. Catches
future refactors that drop a redaction step or route raw data through
a new path.

477 tests pass (was 450); coverage 96%.

https://claude.ai/code/session_01E9jQWivDjJuYMXEFEq9Pq7
Copilot AI review requested due to automatic review settings May 20, 2026 17:26
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR tightens the kernel’s security boundary at the “grant → handle → expand” seam, introduces explicit policy gating for durable memory actions, and adds regression tests that assert secrets/PII never cross the RawResult→Frame/trace boundary.

Changes:

  • Persist grant constraints + principal binding on Handle, and re-check them during HandleStore.expand (stable denial codes via HandleConstraintViolation).
  • Add SensitivityTag.MEMORY policy rules to DefaultPolicyEngine and redact payload-like args in ActionTrace for memory.* capabilities.
  • Add focused “boundary” tests to prevent regressions in firewall redaction, handle-only/raw behavior, and trace arg redaction.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/agent_kernel/handles.py Persists constraints/principal on handles and re-enforces them on expand.
src/agent_kernel/kernel.py Persists principal/constraints into handles; redacts memory args in traces; forwards principal into expand.
src/agent_kernel/policy.py Adds MEMORY policy branch + explain() support with stable reason codes.
src/agent_kernel/policy_reasons.py Introduces new stable denial reason codes for handle+memory paths.
src/agent_kernel/models.py Extends Handle metadata with principal_id and persisted constraints.
src/agent_kernel/errors.py Adds HandleConstraintViolation error with optional reason_code.
src/agent_kernel/enums.py Adds SensitivityTag.MEMORY.
src/agent_kernel/__init__.py Exports HandleConstraintViolation.
tests/test_handles.py Adds tests for grant-constraint enforcement during handle expansion.
tests/test_policy.py Adds tests for memory read/write policy allow/deny behavior and explain() output.
tests/test_firewall_boundary.py New regression suite asserting secrets/PII don’t leak via frames/raw/handles/traces.
docs/security.md Documents handle expansion constraints + memory policies and trace redaction.
docs/architecture.md Updates DefaultPolicyEngine rule ordering + reason-code table.
CHANGELOG.md Documents new security features and tests under Unreleased.

Comment thread src/agent_kernel/handles.py
Comment thread src/agent_kernel/handles.py
Comment thread src/agent_kernel/handles.py
Comment thread docs/security.md Outdated
Comment thread tests/test_handles.py
…put errors

Addresses Copilot review on #79:

- HandleStore.expand: an omitted/empty principal_id now counts as a
  mismatch against a principal-bound handle. Previously the check only
  ran when both sides were non-empty, so a caller could expand by
  passing principal_id="" (or by letting Kernel.expand default to
  principal=None). Docs already claimed handles are not bearer
  credentials — implementation now matches.
- HandleStore.expand: validate query inputs up front and raise
  HandleConstraintViolation (reason_code=INVALID_CONSTRAINT) on a
  non-dict filter, non-list fields, or non-integer offset/limit.
  Public APIs no longer leak bare TypeError/ValueError.
- New regression tests:
  - principal-bound handle expanded without principal_id is denied
    (covers both omitted and explicit "" cases)
  - non-principal-bound handles still expand without a principal
  - invalid filter / fields / offset / limit each surface stable codes
- Updated existing callers (3 tests, 3 examples) to pass principal=
  into kernel.expand for principal-bound handles.
- Tightened Kernel.expand docstring and docs/security.md to call out
  the strict enforcement.

483 tests pass (was 477; +6 for new regression coverage). make ci
green: ruff format/check, mypy strict, pytest, examples.

https://claude.ai/code/session_01E9jQWivDjJuYMXEFEq9Pq7
dgenio pushed a commit that referenced this pull request May 20, 2026
Three review-comment fixes; all three were valid.

examples/tutorial.py:173 — the leakage line printed but never failed
the run. Replaced with `assert leaked == []` so a future firewall
regression on `allowed_fields` enforcement breaks the build (matches
the PR description's "fail loudly" claim).

docs/tutorial.md:170 — removed the false claim that handle expansion
applies projection "on top of allowed_fields, never around it".
Empirically verified the gap: `kernel.expand(handle, query={"fields":
["email"]})` returns the email even when `allowed_fields=["id"]`,
because `kernel.invoke` stores `raw_result.data` unredacted and
`HandleStore.expand` has no knowledge of the original grant's
`allowed_fields`. New paragraph describes today's behavior accurately
and points readers at issue #76 / PR #79 (the in-flight grant-constraint
work) instead of overpromising.

CHANGELOG.md:18 — tutorial only covers the three LLM-safe response
modes (summary / table / handle_only); admin-only `raw` mode is
mentioned but not exercised. Reworded to match.

Verification: ruff format/check clean; pytest 450 passed; tutorial.py
exits 0.

https://claude.ai/code/session_01E9jQWivDjJuYMXEFEq9Pq7
@dgenio dgenio merged commit 8602811 into main May 21, 2026
4 checks passed
@dgenio dgenio deleted the claude/triage-issues-F9tbR branch May 21, 2026 07:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants