diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ecc620..3fbcba2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,56 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added +- Grant-constraint enforcement on handle expansion (#76). `Handle` now carries + the `principal_id` and `constraints` from the original grant, persisted at + handle creation time by `HandleStore.store`. `HandleStore.expand` rechecks + these against the requested expand query: + - A request `limit` larger than the grant's `max_rows` is rejected with + `HandleConstraintViolation` (`reason_code = handle_constraint_violation`). + - A request `fields` entry outside `allowed_fields` is rejected; an + unscoped expand applies `allowed_fields` as the default projection. + - A request filter that disagrees with the grant's `scope` is rejected; + the scope filter is otherwise AND-merged so the caller cannot bypass it. + - A `principal_id` parameter that does not match the handle's stored + principal raises `HandleConstraintViolation` + (`reason_code = handle_principal_mismatch`). +- `SensitivityTag.MEMORY` and memory-action policy rules in + `DefaultPolicyEngine` (#75). Project-scoped memory reads are allowed by + default; sensitive-scoped reads require the `memory_reader_sensitive` role + (or `admin`); writes always require `memory_writer` (or `admin`). The + `explain()` path lists the same conditions with stable `reason_code`s. +- New stable `DenialReason` codes: `HANDLE_CONSTRAINT_VIOLATION`, + `HANDLE_PRINCIPAL_MISMATCH`, `MEMORY_WRITE_REQUIRES_WRITER`, + `MEMORY_SENSITIVE_READ_DENIED`. +- `HandleConstraintViolation` error class (subclass of `AgentKernelError`, + exported from `agent_kernel`) — carries an optional `reason_code` matching + the `DenialReason` vocabulary so handle-side and grant-side denials share + one set of stable codes. +- `Kernel.expand` accepts an optional `principal: Principal` argument that + is forwarded to `HandleStore.expand` for principal-mismatch checks. +- Memory-action input redaction (#75): `ActionTrace.args` for any capability + whose ID starts with `memory.` has payload-like keys (`payload`, `content`, + `value`, `memory`, `text`, `body`) replaced with `"[REDACTED]"`. Keys are + preserved so audit can confirm the action took place without exposing the + durable content the agent wrote or read. +- New `tests/test_firewall_boundary.py` (#74) — focused regression suite that + pushes synthetic secret/PII values through the raw → `Frame` boundary + end-to-end and asserts those values never appear in summary/table/raw + frames, are stripped by `allowed_fields`, never reach `ActionTrace.args` + for memory capabilities, and stay quarantined when raw mode is downgraded + for non-admin principals. + +### Security +- Closes #76: handle expansion can no longer return data outside the original + grant's `max_rows` / `allowed_fields` / `scope`, and handle IDs are no + longer bearer credentials that work across principals. +- Closes #75: memory reads and writes are governed actions with stable + denial codes and trace-side redaction of durable payloads. +- Closes #74: redaction boundary is pinned by negative assertions against + fake-secret strings, catching future regressions that drop a redaction + step or route raw data through a new path. + ## [0.7.0] - 2026-05-20 ### Added diff --git a/docs/architecture.md b/docs/architecture.md index 55334bd..2806812 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -52,8 +52,9 @@ Both built-in engines satisfy `ExplainingPolicyEngine`: 3. **DESTRUCTIVE** — requires role `admin` + `justification ≥ 15 chars` 4. **PII/PCI** — requires `tenant` attribute; enforces `allowed_fields` unless `pii_reader` 5. **SECRETS** — requires role `admin|secrets_reader` + `justification ≥ 15 chars` - 6. **max_rows** — 50 (user), 500 (service) - 7. **Rate limiting** — sliding-window per `(principal_id, capability_id)` (60 READ / 10 WRITE / 2 DESTRUCTIVE per 60s; service role gets 10×) + 6. **MEMORY** — `memory.read` with `scope.memory_scope == "sensitive"` requires role `memory_reader_sensitive|admin`; `memory.write` / DESTRUCTIVE memory requires role `memory_writer|admin`. Project-scoped memory reads are allowed by default. The kernel also redacts `payload`/`content`/`value`/`memory`/`text`/`body` keys from `ActionTrace.args` for any capability whose ID starts with `memory.` + 7. **max_rows** — 50 (user), 500 (service) + 8. **Rate limiting** — sliding-window per `(principal_id, capability_id)` (60 READ / 10 WRITE / 2 DESTRUCTIVE per 60s; service role gets 10×) - **`DeclarativePolicyEngine`** — loads rules from a YAML or TOML file (or a plain dict). Supports `safety_class`, `sensitivity`, `roles`, `attributes`, `min_justification`, `intent`, and `scope` match conditions; `allow`/`deny` actions; per-rule `constraints` merged into the resulting `PolicyDecision`; configurable `default` action. Rules are evaluated top-down with first-match-wins. `pyyaml` and `tomli` are optional dependencies — `import agent_kernel` works without them; calling `from_yaml`/`from_toml` without the parser raises `PolicyConfigError` with an install hint. #### Intent and scope on requests @@ -96,6 +97,10 @@ Every `PolicyDecision`, `DenialExplanation`, `FailedCondition`, and `PolicyDenie | `explicit_deny_rule` | DSL: a `deny` rule matched fully | | `intent_not_allowed` | DSL: `match.intent` rejected the request's intent | | `scope_not_allowed` | DSL: `match.scope` rejected the request's scope | +| `handle_constraint_violation` | `HandleStore.expand` request exceeded grant's `max_rows`, `allowed_fields`, or `scope` (#76) | +| `handle_principal_mismatch` | Handle expansion attempted by a different principal than the one the original grant was issued to (#76) | +| `memory_write_requires_writer` | `SensitivityTag.MEMORY` WRITE/DESTRUCTIVE without `memory_writer` or `admin` role (#75) | +| `memory_sensitive_read_denied` | `SensitivityTag.MEMORY` read with `scope.memory_scope == "sensitive"` without `memory_reader_sensitive` or `admin` role (#75) | Allow-side codes (`AllowReason.*`): `default_policy_allow`, `rule_allow`, `default_fallthrough_allow`, `token_verified`. diff --git a/docs/security.md b/docs/security.md index 8343157..880eb50 100644 --- a/docs/security.md +++ b/docs/security.md @@ -12,6 +12,9 @@ | PII / PCI leakage | Redaction + `allowed_fields` enforcement in the firewall | | Privilege escalation via WRITE/DESTRUCTIVE | Policy engine enforces role requirements | | Audit evasion | Every `invoke()` creates an immutable `ActionTrace` | +| Handle scope escape (expand exceeds grant) | Handles persist grant constraints; `HandleStore.expand` rechecks `max_rows`, `allowed_fields`, `scope`, and principal binding (#76) | +| Memory exfiltration via tool output | `SensitivityTag.MEMORY` capabilities gate sensitive reads and durable writes; `ActionTrace.args` redacts payload-like fields for `memory.*` capabilities (#75) | +| Raw memory payload reaching audit log | Kernel strips `payload`/`content`/`value`/`memory`/`text`/`body` from `ActionTrace.args` for `memory.*` capabilities | ## Token scopes @@ -27,6 +30,56 @@ Any change to these fields invalidates the HMAC signature. Consider an agent that obtains a token for `billing.list_invoices` then passes it to a different agent. The second agent cannot use it because `verify()` checks that `token.principal_id == expected_principal_id`. +The same principle extends to handles: every `Handle` carries the `principal_id` +the original grant was issued to. When `handle.principal_id` is non-empty, +`HandleStore.expand` rejects expansion unless the caller supplies a matching +`principal_id`. **An omitted or empty `principal_id` is treated as a +mismatch** (`HandleConstraintViolation`, `reason_code = HANDLE_PRINCIPAL_MISMATCH`), +so a handle ID alone is not a bearer credential — proof of the original +principal is always required. `Kernel.expand(..., principal=Principal(...))` +forwards the principal automatically. + +## Handle expansion boundary + +Calling `kernel.expand(handle, query=...)` does not re-run the policy engine — +the original grant already authorised the dataset, and handles are short-lived. +But the grant's _constraints_ must still apply, otherwise an over-broad +`expand` query would silently return data the original grant never covered. + +`HandleStore.expand` rechecks the constraints the kernel persists on the handle +at creation time (`token.constraints`): + +| Constraint | Enforced behavior on expand | +|------------|-----------------------------| +| `max_rows` | A request `limit` larger than the cap raises `HandleConstraintViolation`. An unspecified or larger implicit limit is silently clamped. | +| `allowed_fields` | A request `fields` entry that is not in `allowed_fields` raises `HandleConstraintViolation`. An unscoped expand applies `allowed_fields` as the default projection, so disallowed fields never leak. | +| `scope` (e.g. `{"region": "eu"}`) | The scope filter is AND-merged into the request filter. A request filter that disagrees on a scoped dimension raises `HandleConstraintViolation`. | +| `principal_id` | A mismatched `principal_id` parameter raises `HandleConstraintViolation` (`HANDLE_PRINCIPAL_MISMATCH`). | + +Errors carry stable `reason_code` values (`handle_constraint_violation`, +`handle_principal_mismatch`) — assert on those, not on the message text. + +## Memory actions + +Capabilities tagged `SensitivityTag.MEMORY` represent durable agent memory +(project notes, session handoff, learned context). Reads of project-scoped +memory are allowed by default; reads of sensitive-scoped memory require an +explicit role. Writes always require the `memory_writer` role (or `admin`) +because they persist into future sessions. + +| Action | Required role | Denial reason code | +|--------|---------------|--------------------| +| `memory.read` with `scope["memory_scope"] == "project"` | none | — | +| `memory.read` with `scope["memory_scope"] == "sensitive"` | `memory_reader_sensitive` or `admin` | `memory_sensitive_read_denied` | +| `memory.write` (any scope) | `memory_writer` or `admin` | `memory_write_requires_writer` | +| `memory.forget` (DESTRUCTIVE) | `admin` (then `memory_writer` or `admin`) | `missing_role`, then `memory_write_requires_writer` | + +To prevent durable memory content from leaking into the audit log, the kernel +strips payload-like fields (`payload`, `content`, `value`, `memory`, `text`, +`body`) from `ActionTrace.args` for any capability whose ID begins with +`memory.`. Non-sensitive metadata keys (`key`, `id`, `scope`, ...) are +preserved so audit can still confirm an action took place. + ## Security disclaimers > **v0.1 is not production-hardened for real authentication.** diff --git a/examples/basic_cli.py b/examples/basic_cli.py index 507176f..efad0ce 100644 --- a/examples/basic_cli.py +++ b/examples/basic_cli.py @@ -124,6 +124,7 @@ async def main() -> None: expanded = kernel.expand( frame.handle, query={"offset": 0, "limit": 3, "fields": ["id", "title"]}, + principal=reader, ) print(" First 3 rows (id + title only):") for row in expanded.table_preview: diff --git a/examples/billing_demo.py b/examples/billing_demo.py index d0c1008..7b2f8fa 100644 --- a/examples/billing_demo.py +++ b/examples/billing_demo.py @@ -111,6 +111,7 @@ async def main() -> None: expanded = kernel.expand( frame.handle, query={"offset": 0, "limit": 3, "fields": ["id", "amount", "status"]}, + principal=analyst, ) for row in expanded.table_preview: print(f" {row}") @@ -121,6 +122,7 @@ async def main() -> None: overdue = kernel.expand( frame.handle, query={"filter": {"status": "overdue"}, "limit": 3, "fields": ["id", "amount"]}, + principal=analyst, ) print(f" Overdue rows returned: {len(overdue.table_preview)}") for row in overdue.table_preview: diff --git a/examples/http_driver_demo.py b/examples/http_driver_demo.py index e23b577..a3ce571 100644 --- a/examples/http_driver_demo.py +++ b/examples/http_driver_demo.py @@ -119,6 +119,7 @@ async def main() -> None: expanded = kernel.expand( frame.handle, query={"limit": 3, "fields": ["id", "name", "price"]}, + principal=principal, ) for row in expanded.table_preview: print(f" {row}") diff --git a/src/agent_kernel/__init__.py b/src/agent_kernel/__init__.py index e7449b5..e99cab7 100644 --- a/src/agent_kernel/__init__.py +++ b/src/agent_kernel/__init__.py @@ -57,6 +57,7 @@ CapabilityNotFound, DriverError, FirewallError, + HandleConstraintViolation, HandleExpired, HandleNotFound, PolicyConfigError, @@ -143,6 +144,7 @@ "CapabilityNotFound", "DriverError", "FirewallError", + "HandleConstraintViolation", "HandleExpired", "HandleNotFound", "PolicyConfigError", diff --git a/src/agent_kernel/enums.py b/src/agent_kernel/enums.py index c14da31..6bc7ee6 100644 --- a/src/agent_kernel/enums.py +++ b/src/agent_kernel/enums.py @@ -30,3 +30,11 @@ class SensitivityTag(str, Enum): SECRETS = "SECRETS" """Credentials, API keys, tokens.""" + + MEMORY = "MEMORY" + """Durable agent memory (project notes, session handoff, learned context). + + Reading durable memory may expose sensitive past context; writing creates + durable assumptions that persist into future sessions. Policy treats + writes as higher risk than reads. See ``DefaultPolicyEngine.evaluate``. + """ diff --git a/src/agent_kernel/errors.py b/src/agent_kernel/errors.py index 7944f7b..aae15c0 100644 --- a/src/agent_kernel/errors.py +++ b/src/agent_kernel/errors.py @@ -126,3 +126,22 @@ class HandleNotFound(AgentKernelError): class HandleExpired(AgentKernelError): """Raised when a handle's TTL has elapsed.""" + + +class HandleConstraintViolation(AgentKernelError): + """Raised when a handle expansion request violates the grant's constraints. + + Handles persist the constraints attached to the original + :class:`~agent_kernel.models.PolicyDecision` (e.g. ``max_rows``, + ``allowed_fields``). :meth:`HandleStore.expand` rechecks the requested + query against those constraints; expansions that would exceed the row + cap, request disallowed fields, or violate scope raise this error so + callers can branch on ``reason_code`` rather than parse the message. + + Carries the same ``reason_code`` shape as :class:`PolicyDenied` so + metrics and UI mapping use one denial vocabulary across the kernel. + """ + + def __init__(self, message: str, *, reason_code: str | None = None) -> None: + super().__init__(message) + self.reason_code: str | None = reason_code diff --git a/src/agent_kernel/handles.py b/src/agent_kernel/handles.py index 2f4b8e9..7530e2b 100644 --- a/src/agent_kernel/handles.py +++ b/src/agent_kernel/handles.py @@ -6,8 +6,9 @@ import uuid from typing import Any -from .errors import HandleExpired, HandleNotFound +from .errors import HandleConstraintViolation, HandleExpired, HandleNotFound from .models import Frame, Handle, Provenance, ResponseMode +from .policy_reasons import DenialReason class HandleStore: @@ -41,6 +42,8 @@ def store( data: Any, *, ttl_seconds: int | None = None, + principal_id: str = "", + constraints: dict[str, Any] | None = None, ) -> Handle: """Store *data* and return a :class:`Handle`. @@ -48,6 +51,12 @@ def store( capability_id: The capability that produced *data*. data: The full dataset to store. ttl_seconds: Time-to-live in seconds (defaults to the store default). + principal_id: Principal the original grant was issued to. When + non-empty, :meth:`expand` rejects requests from other + principals with :class:`HandleConstraintViolation`. + constraints: Grant constraints to persist on the handle (typically + ``token.constraints`` — e.g. ``max_rows``, ``allowed_fields``, + ``scope``). :meth:`expand` rechecks these. Returns: A :class:`Handle` referencing the stored data. @@ -60,6 +69,8 @@ def store( created_at=now, expires_at=now + datetime.timedelta(seconds=ttl), total_rows=len(data) if isinstance(data, list) else 1, + principal_id=principal_id, + constraints=dict(constraints) if constraints else {}, ) self._data[handle.handle_id] = data self._meta[handle.handle_id] = handle @@ -135,6 +146,7 @@ def expand( query: dict[str, Any], action_id: str = "", response_mode: ResponseMode = "table", + principal_id: str = "", ) -> Frame: """Expand a handle with optional pagination, field selection, and filtering. @@ -149,6 +161,8 @@ def expand( query: Query parameters controlling the expansion. action_id: Audit action ID to embed in the returned Frame. response_mode: Response mode for the returned Frame. + principal_id: Principal performing the expansion. When the handle + was created with a non-empty ``principal_id``, this must match. Returns: A :class:`Frame` containing the slice of data. @@ -156,13 +170,78 @@ def expand( Raises: HandleNotFound: If the handle ID is unknown. HandleExpired: If the handle's TTL has elapsed. + HandleConstraintViolation: If the requested expansion exceeds the + grant's persisted constraints (``max_rows``, ``allowed_fields``, + ``scope``) or is requested by a different principal. """ + # ── Principal binding ───────────────────────────────────────────────── + # A handle bound to a principal is not a bearer credential. If the + # handle has a non-empty principal_id, the caller MUST present a + # matching principal_id — omitting it counts as a mismatch (otherwise + # any caller in the same process could expand by passing ""). + if handle.principal_id and handle.principal_id != principal_id: + raise HandleConstraintViolation( + f"Handle '{handle.handle_id}' was granted to principal " + f"'{handle.principal_id}' and cannot be expanded by " + f"'{principal_id or ''}'.", + reason_code=DenialReason.HANDLE_PRINCIPAL_MISMATCH, + ) + + # ── Query input validation ──────────────────────────────────────────── + # Validate user-supplied query types up front so callers see a stable + # HandleConstraintViolation (with INVALID_CONSTRAINT) instead of a + # bare TypeError/ValueError from inside the expansion logic. Public + # interfaces must not leak stdlib exceptions (see AGENTS.md). + raw_filter = query.get("filter") + if raw_filter is not None and not isinstance(raw_filter, dict): + raise HandleConstraintViolation( + f"Handle expand 'filter' must be a dict, got {type(raw_filter).__name__}.", + reason_code=DenialReason.INVALID_CONSTRAINT, + ) + raw_fields = query.get("fields") + if raw_fields is not None and not isinstance(raw_fields, list | tuple): + raise HandleConstraintViolation( + f"Handle expand 'fields' must be a list, got {type(raw_fields).__name__}.", + reason_code=DenialReason.INVALID_CONSTRAINT, + ) + + # ── Grant-constraint rechecks ───────────────────────────────────────── + granted_max_rows = handle.constraints.get("max_rows") + granted_fields = handle.constraints.get("allowed_fields") or [] + granted_scope = handle.constraints.get("scope") or {} + + requested_fields: list[str] = list(raw_fields or []) + if granted_fields and requested_fields: + disallowed = [f for f in requested_fields if f not in granted_fields] + if disallowed: + raise HandleConstraintViolation( + f"Handle '{handle.handle_id}' grant restricts fields to " + f"{sorted(granted_fields)}; request asked for " + f"{sorted(disallowed)}.", + reason_code=DenialReason.HANDLE_CONSTRAINT_VIOLATION, + ) + + filter_in: dict[str, Any] = raw_filter or {} + if granted_scope: + for sk, sv in granted_scope.items(): + if sk in filter_in and filter_in[sk] != sv: + raise HandleConstraintViolation( + f"Handle '{handle.handle_id}' is scoped to " + f"{sk}={sv!r}; request filter " + f"{sk}={filter_in[sk]!r} is outside that scope.", + reason_code=DenialReason.HANDLE_CONSTRAINT_VIOLATION, + ) + data = self.get(handle.handle_id) rows: list[Any] = data if isinstance(data, list) else [data] # ── Filtering ────────────────────────────────────────────────────────── - filter_spec: dict[str, Any] = query.get("filter", {}) - if filter_spec and isinstance(filter_spec, dict): + # Grant scope is AND-merged into the request filter so the caller + # cannot bypass it by omitting the scope key. + filter_spec: dict[str, Any] = dict(filter_in) + for sk, sv in granted_scope.items(): + filter_spec.setdefault(sk, sv) + if filter_spec: rows = [ r for r in rows @@ -170,15 +249,53 @@ def expand( ] # ── Pagination ──────────────────────────────────────────────────────── - offset = int(query.get("offset", 0)) - limit = int(query.get("limit", len(rows))) + try: + offset = int(query.get("offset", 0)) + except (TypeError, ValueError) as exc: + raise HandleConstraintViolation( + f"Handle expand 'offset' must be an integer, got {query.get('offset')!r}.", + reason_code=DenialReason.INVALID_CONSTRAINT, + ) from exc + requested_limit_raw = query.get("limit") + requested_limit: int | None + if requested_limit_raw is None: + requested_limit = None + else: + try: + requested_limit = int(requested_limit_raw) + except (TypeError, ValueError) as exc: + raise HandleConstraintViolation( + f"Handle expand 'limit' must be an integer, got {requested_limit_raw!r}.", + reason_code=DenialReason.INVALID_CONSTRAINT, + ) from exc + limit = len(rows) if requested_limit is None else requested_limit + + if isinstance(granted_max_rows, int) and granted_max_rows >= 0: + if requested_limit is not None and requested_limit > granted_max_rows: + raise HandleConstraintViolation( + f"Handle '{handle.handle_id}' grant caps rows at " + f"{granted_max_rows}; request asked for " + f"limit={requested_limit}.", + reason_code=DenialReason.HANDLE_CONSTRAINT_VIOLATION, + ) + limit = min(limit, granted_max_rows) + rows = rows[offset : offset + limit] # ── Field selection ─────────────────────────────────────────────────── - fields: list[str] = list(query.get("fields", [])) - if fields: + # If the grant restricts fields and the caller did not ask for any, + # apply the grant's allowed_fields as the default projection so + # disallowed fields cannot leak through an unscoped expand call. + effective_fields: list[str] + if requested_fields: + effective_fields = requested_fields + elif granted_fields: + effective_fields = list(granted_fields) + else: + effective_fields = [] + if effective_fields: rows = [ - {k: v for k, v in r.items() if k in fields} if isinstance(r, dict) else r + {k: v for k, v in r.items() if k in effective_fields} if isinstance(r, dict) else r for r in rows ] diff --git a/src/agent_kernel/kernel.py b/src/agent_kernel/kernel.py index d8cc3bf..521dd63 100644 --- a/src/agent_kernel/kernel.py +++ b/src/agent_kernel/kernel.py @@ -39,6 +39,36 @@ logger = logging.getLogger(__name__) +_MEMORY_CAPABILITY_PREFIX = "memory." +_MEMORY_SENSITIVE_ARG_KEYS: frozenset[str] = frozenset( + {"payload", "content", "value", "memory", "text", "body"} +) + + +def _redact_args_for_trace(capability_id: str, args: dict[str, Any]) -> dict[str, Any]: + """Strip raw memory payloads from :class:`ActionTrace.args`. + + Memory capabilities (``capability_id`` starting with ``"memory."``) may + carry durable text the principal is committing to or fetching from + long-term memory. Tracing the raw payload would defeat the I-01 boundary + that the :class:`Firewall` enforces for outputs — so we apply an + equivalent input-side redaction at trace-record time. + + The resulting dict preserves keys (so audit can confirm a payload was + provided) and replaces sensitive values with ``"[REDACTED]"``. Non-memory + capabilities are returned unchanged. + """ + if not capability_id.startswith(_MEMORY_CAPABILITY_PREFIX): + return args + redacted: dict[str, Any] = {} + for k, v in args.items(): + if k.lower() in _MEMORY_SENSITIVE_ARG_KEYS: + redacted[k] = "[REDACTED]" + else: + redacted[k] = v + return redacted + + def _frame_payload(frame: Frame) -> Any: """Return the LLM-facing payload from a :class:`Frame` for token counting. @@ -425,7 +455,7 @@ async def invoke( principal_id=principal.principal_id, token_id=token.token_id, invoked_at=datetime.datetime.now(tz=datetime.timezone.utc), - args=args, + args=_redact_args_for_trace(token.capability_id, args), response_mode=response_mode, driver_id="", error=err_msg, @@ -441,6 +471,8 @@ async def invoke( handle = self._handle_store.store( capability_id=token.capability_id, data=raw_result.data, + principal_id=principal.principal_id, + constraints=token.constraints, ) # ── Firewall transform + budget reconciliation ──────────────────────── @@ -478,7 +510,7 @@ async def invoke( principal_id=principal.principal_id, token_id=token.token_id, invoked_at=datetime.datetime.now(tz=datetime.timezone.utc), - args=args, + args=_redact_args_for_trace(token.capability_id, args), response_mode=frame.response_mode, driver_id=used_driver_id, handle_id=handle.handle_id if handle else None, @@ -491,12 +523,23 @@ async def invoke( ) return frame - def expand(self, handle: Handle, *, query: dict[str, Any]) -> Frame: + def expand( + self, + handle: Handle, + *, + query: dict[str, Any], + principal: Principal | None = None, + ) -> Frame: """Expand a handle with pagination, field selection, or filtering. Args: handle: The :class:`Handle` to expand. query: Query parameters (``offset``, ``limit``, ``fields``, ``filter``). + principal: The principal performing the expansion. **Required** + when the handle was created with a non-empty ``principal_id``: + an omitted principal is treated as a mismatch and raises + :class:`HandleConstraintViolation` (handle IDs are not bearer + credentials). Optional for handles that were not principal-bound. Returns: A :class:`Frame` with the requested slice of data. @@ -504,15 +547,23 @@ def expand(self, handle: Handle, *, query: dict[str, Any]) -> Frame: Raises: HandleNotFound: If the handle is unknown. HandleExpired: If the handle has expired. + HandleConstraintViolation: If the requested expansion exceeds the + grant's persisted constraints (``max_rows``, ``allowed_fields``, + ``scope``) or is requested by a different principal. """ logger.info( "expand", extra={ "handle_id": handle.handle_id, "capability_id": handle.capability_id, + "principal_id": principal.principal_id if principal else "", }, ) - return self._handle_store.expand(handle, query=query) + return self._handle_store.expand( + handle, + query=query, + principal_id=principal.principal_id if principal else "", + ) def explain(self, action_id: str) -> ActionTrace: """Retrieve the audit trace for a past invocation. diff --git a/src/agent_kernel/models.py b/src/agent_kernel/models.py index 9c6adea..02b6b8c 100644 --- a/src/agent_kernel/models.py +++ b/src/agent_kernel/models.py @@ -314,7 +314,14 @@ class RawResult: @dataclass(slots=True) class Handle: - """An opaque reference to a full dataset stored in the HandleStore.""" + """An opaque reference to a full dataset stored in the HandleStore. + + Handles carry the grant constraints persisted at creation time. The + :class:`HandleStore` rechecks those constraints when the handle is + expanded, so an over-broad expand query is denied with a stable + :class:`~agent_kernel.errors.HandleConstraintViolation` rather than + silently returning data the original grant never authorised. + """ handle_id: str capability_id: str @@ -322,6 +329,16 @@ class Handle: expires_at: datetime.datetime total_rows: int = 0 + principal_id: str = "" + """Principal the original grant was issued to. ``expand`` rejects use by + other principals so handle references cannot be shared as a back-door.""" + + constraints: dict[str, Any] = field(default_factory=dict) + """Grant constraints copied from :attr:`PolicyDecision.constraints` at + handle creation time. ``expand`` rechecks ``max_rows``, ``allowed_fields``, + and any ``scope`` filter against these values. + """ + @dataclass(slots=True) class Provenance: diff --git a/src/agent_kernel/policy.py b/src/agent_kernel/policy.py index 5a3e9f7..7a7645e 100644 --- a/src/agent_kernel/policy.py +++ b/src/agent_kernel/policy.py @@ -399,6 +399,48 @@ def _record_deny(detail: str, code: str) -> None: reason_code=DenialReason.INSUFFICIENT_JUSTIFICATION, ) + # ── Memory action checks ───────────────────────────────────────────── + # Placed AFTER all other sensitivity checks (see invariants.md: + # "rule placement matters"). Memory reads at scope == "sensitive" + # require an explicit reader role; memory writes are treated as + # higher-risk than reads because they persist into future sessions + # and require the 'memory_writer' role (or 'admin'). + if capability.sensitivity == SensitivityTag.MEMORY: + memory_scope = str(request.scope.get("memory_scope", "")) if request.scope else "" + is_write = capability.safety_class in ( + SafetyClass.WRITE, + SafetyClass.DESTRUCTIVE, + ) + if is_write and not (roles & {"memory_writer", "admin"}): + detail = ( + f"MEMORY write capabilities require the 'memory_writer' or " + f"'admin' role. Principal '{pid}' has roles: {sorted(roles)}." + ) + _record_deny(detail, DenialReason.MEMORY_WRITE_REQUIRES_WRITER) + raise self._deny( + detail, + principal_id=pid, + capability_id=cid, + reason_code=DenialReason.MEMORY_WRITE_REQUIRES_WRITER, + ) + if ( + not is_write + and memory_scope == "sensitive" + and not (roles & {"memory_reader_sensitive", "admin"}) + ): + detail = ( + f"MEMORY read with scope='sensitive' requires the " + f"'memory_reader_sensitive' or 'admin' role. " + f"Principal '{pid}' has roles: {sorted(roles)}." + ) + _record_deny(detail, DenialReason.MEMORY_SENSITIVE_READ_DENIED) + raise self._deny( + detail, + principal_id=pid, + capability_id=cid, + reason_code=DenialReason.MEMORY_SENSITIVE_READ_DENIED, + ) + # ── Row cap ─────────────────────────────────────────────────────────── max_rows = _MAX_ROWS_SERVICE if "service" in roles else _MAX_ROWS_USER @@ -603,6 +645,41 @@ def explain( ) ) + if capability.sensitivity == SensitivityTag.MEMORY: + memory_scope = str(request.scope.get("memory_scope", "")) if request.scope else "" + is_write = capability.safety_class in ( + SafetyClass.WRITE, + SafetyClass.DESTRUCTIVE, + ) + if is_write and not (roles & {"memory_writer", "admin"}): + failed.append( + FailedCondition( + condition="roles", + required=["memory_writer", "admin"], + actual=sorted(roles), + suggestion=(f"Add 'memory_writer' or 'admin' role to principal '{pid}'"), + reason_code=str(DenialReason.MEMORY_WRITE_REQUIRES_WRITER), + ) + ) + if ( + not is_write + and memory_scope == "sensitive" + and not (roles & {"memory_reader_sensitive", "admin"}) + ): + failed.append( + FailedCondition( + condition="roles", + required=["memory_reader_sensitive", "admin"], + actual=sorted(roles), + suggestion=( + f"Add 'memory_reader_sensitive' or 'admin' role to " + f"principal '{pid}' (or narrow the request scope away " + f"from 'sensitive')" + ), + reason_code=str(DenialReason.MEMORY_SENSITIVE_READ_DENIED), + ) + ) + denied = bool(failed) remediation = [fc.suggestion for fc in failed] diff --git a/src/agent_kernel/policy_reasons.py b/src/agent_kernel/policy_reasons.py index 3c1f82d..af318e8 100644 --- a/src/agent_kernel/policy_reasons.py +++ b/src/agent_kernel/policy_reasons.py @@ -72,6 +72,31 @@ class DenialReason(_StrEnumCompat): SCOPE_NOT_ALLOWED = "scope_not_allowed" """A declarative rule restricted scope values and the request's scope does not match.""" + # Handle expansion (#76) + HANDLE_CONSTRAINT_VIOLATION = "handle_constraint_violation" + """A handle expansion request violated the grant's persisted constraints + (e.g. requested ``limit`` exceeds the grant's ``max_rows``, requested + fields are not in ``allowed_fields``, or the scope filter is incompatible). + """ + + HANDLE_PRINCIPAL_MISMATCH = "handle_principal_mismatch" + """A handle was presented for expansion by a principal other than the one + the original grant was issued to. Handles are principal-scoped to prevent + confused-deputy through handle sharing. + """ + + # Memory actions (#75) + MEMORY_WRITE_REQUIRES_WRITER = "memory_write_requires_writer" + """A ``memory.write`` capability was requested without the + ``memory_writer`` role. Durable memory writes persist into future + sessions and require an explicit role grant. + """ + + MEMORY_SENSITIVE_READ_DENIED = "memory_sensitive_read_denied" + """A memory read of scope ``sensitive`` was requested without the + ``memory_reader_sensitive`` role. + """ + class AllowReason(_StrEnumCompat): """Stable codes describing why a policy decision allowed a request.""" diff --git a/tests/test_firewall_boundary.py b/tests/test_firewall_boundary.py new file mode 100644 index 0000000..e987b4f --- /dev/null +++ b/tests/test_firewall_boundary.py @@ -0,0 +1,274 @@ +"""Boundary regression tests for the raw → Frame / Handle / ActionTrace seam (#74). + +These tests pin the I-01 contract from a different angle than ``test_firewall.py``: +they construct synthetic records with obviously-fake secret and PII-like values, +push them through the firewall and kernel, then assert *negatively* — those +values must not appear anywhere a downstream consumer (LLM, audit reader, log +sink) is expected to look. + +The intent is to catch a future refactor that quietly drops a redaction step +or routes raw data through a new path. New response modes or trace fields +should add a case here. +""" + +from __future__ import annotations + +import asyncio +import datetime +import json +from typing import Any + +import pytest + +from agent_kernel import ( + Capability, + CapabilityRegistry, + HandleStore, + HMACTokenProvider, + InMemoryDriver, + Kernel, + Principal, + SafetyClass, + SensitivityTag, + StaticRouter, + TraceStore, +) +from agent_kernel.firewall.transform import Firewall +from agent_kernel.models import CapabilityRequest, Handle, RawResult + +# Fake values — these strings exist nowhere except this test file. Any test +# failure that prints them is loud about which boundary leaked. +_FAKE_EMAIL = "alice.fake-victim@example.invalid" +_FAKE_BEARER = "Bearer fake-bearer-zzz-9999-do-not-use" +_FAKE_CARD = "4111 1111 1111 1234" +_FAKE_SSN = "123-45-6789" +_FAKE_API_KEY_VALUE = "ZZZZ_FAKEKEY_ABCDEF12345" +_FAKE_INTERNAL_NOTE = "INTERNAL-ONLY-zzz-do-not-leak" + + +def _record() -> dict[str, Any]: + """A synthetic 'customer' row carrying every field type we redact.""" + return { + "id": 1, + "name": "Alice Public", + "email": _FAKE_EMAIL, + "ssn": _FAKE_SSN, + "card_number": _FAKE_CARD, + "authorization": _FAKE_BEARER, + "secret_handshake": f"api_key={_FAKE_API_KEY_VALUE}", + "internal_notes": _FAKE_INTERNAL_NOTE, + "public_status": "active", + } + + +def _handle() -> Handle: + now = datetime.datetime.now(tz=datetime.timezone.utc) + return Handle( + handle_id="bh1", + capability_id="cap.customers", + created_at=now, + expires_at=now + datetime.timedelta(hours=1), + total_rows=1, + ) + + +def _frame_text(frame: object) -> str: + """Serialize the entire frame to JSON for global negative assertions.""" + return json.dumps(frame, default=lambda o: getattr(o, "__dict__", str(o))) + + +# ── Raw → Frame redaction boundary ──────────────────────────────────────────── + + +@pytest.mark.parametrize("mode", ["summary", "table"]) +def test_inline_secrets_never_reach_default_modes(mode: str) -> None: + fw = Firewall() + raw = RawResult(capability_id="cap.customers", data=[_record()]) + frame = fw.transform( + raw, + action_id="a1", + principal_id="u1", + principal_roles=["reader"], + response_mode=mode, # type: ignore[arg-type] + handle=_handle(), + ) + serialized = _frame_text(frame) + for needle in (_FAKE_BEARER, _FAKE_API_KEY_VALUE, _FAKE_SSN, _FAKE_CARD): + assert needle not in serialized, ( + f"{mode}: secret/PII pattern {needle!r} leaked through firewall" + ) + + +def test_allowed_fields_strips_disallowed_columns() -> None: + """When allowed_fields constrains output, the disallowed column is dropped + even if it contained otherwise-benign text.""" + fw = Firewall() + raw = RawResult(capability_id="cap.customers", data=[_record()]) + frame = fw.transform( + raw, + action_id="a2", + principal_id="u1", + principal_roles=["reader"], + response_mode="table", + constraints={"allowed_fields": ["id", "public_status"]}, + handle=_handle(), + ) + serialized = _frame_text(frame) + assert _FAKE_INTERNAL_NOTE not in serialized, "internal_notes leaked" + assert _FAKE_EMAIL not in serialized, "email leaked" + # And no remnants of the disallowed *key names* in the table rows either. + for row in frame.table_preview: + assert "internal_notes" not in row + assert "email" not in row + + +def test_handle_only_mode_drops_raw_payload() -> None: + """handle_only frames carry a Handle reference but never the raw rows.""" + fw = Firewall() + raw = RawResult(capability_id="cap.customers", data=[_record()]) + frame = fw.transform( + raw, + action_id="a3", + principal_id="u1", + principal_roles=["reader"], + response_mode="handle_only", + handle=_handle(), + ) + assert frame.handle is not None, "handle_only must carry a handle reference" + assert frame.table_preview == [], "handle_only must not include row preview" + assert frame.raw_data is None, "handle_only must not include raw data" + assert frame.facts == [], "handle_only must not include facts" + + +def test_raw_mode_downgrades_for_non_admin() -> None: + """A non-admin asking for raw must NOT receive raw data — the firewall + transparently downgrades to summary. This pins invariant I-01 for raw.""" + fw = Firewall() + raw = RawResult(capability_id="cap.customers", data=[_record()]) + frame = fw.transform( + raw, + action_id="a4", + principal_id="u1", + principal_roles=["reader"], + response_mode="raw", + handle=_handle(), + ) + assert frame.response_mode == "summary", "raw must downgrade to summary for non-admin" + assert frame.raw_data is None, "raw_data must remain unpopulated after downgrade" + serialized = _frame_text(frame) + for needle in (_FAKE_BEARER, _FAKE_API_KEY_VALUE, _FAKE_SSN): + assert needle not in serialized + + +def test_raw_mode_admin_carries_raw_data_redacted() -> None: + """An admin in raw mode does see raw data — but inline secret patterns + are still scrubbed because redact() always runs first.""" + fw = Firewall() + raw = RawResult(capability_id="cap.customers", data=[_record()]) + frame = fw.transform( + raw, + action_id="a5", + principal_id="u1", + principal_roles=["admin"], + response_mode="raw", + handle=_handle(), + ) + assert frame.response_mode == "raw", "admin keeps raw mode" + assert frame.raw_data is not None + serialized = _frame_text(frame.raw_data) + # Bearer / api_key / JWT-shaped tokens are matched by the redaction regex + # set, so even raw-mode admin output should not contain them verbatim. + assert _FAKE_BEARER not in serialized + assert _FAKE_API_KEY_VALUE not in serialized + + +# ── Kernel-level: ActionTrace must not leak memory payloads (#75) ───────────── + + +def _build_kernel() -> tuple[Kernel, Principal, Capability]: + registry = CapabilityRegistry() + cap = Capability( + capability_id="memory.write", + name="memory write", + description="store agent note", + safety_class=SafetyClass.WRITE, + sensitivity=SensitivityTag.MEMORY, + ) + registry.register(cap) + driver = InMemoryDriver(driver_id="memory") + driver.register_handler("memory.write", lambda _ctx: [{"ok": True}]) + router = StaticRouter(routes={"memory.write": ["memory"]}) + kernel = Kernel( + registry=registry, + token_provider=HMACTokenProvider(secret="test-secret-do-not-use-in-prod"), + router=router, + handle_store=HandleStore(), + trace_store=TraceStore(), + ) + kernel.register_driver(driver) + return kernel, Principal(principal_id="agent-1", roles=["admin"]), cap + + +def test_action_trace_redacts_memory_payload_arg() -> None: + """ActionTrace.args must not contain raw memory payloads for memory.* caps.""" + kernel, principal, cap = _build_kernel() + request = CapabilityRequest( + capability_id=cap.capability_id, goal="store a durable note for later" + ) + grant = kernel.grant_capability( + request, principal, justification="agent learned a project convention to remember" + ) + + payload = _FAKE_INTERNAL_NOTE # the "memory content" the agent wants to store + asyncio.run( + kernel.invoke( + grant.token, + principal=principal, + args={"payload": payload, "key": "project_convention"}, + ) + ) + trace_list = [t for t in kernel._trace_store.list_all() if t.capability_id == "memory.write"] + assert trace_list, "expected an ActionTrace for the memory.write invocation" + trace = trace_list[-1] + # The payload (raw memory content) must have been redacted... + assert trace.args.get("payload") == "[REDACTED]" + # ...while the non-sensitive metadata key is preserved for audit value. + assert trace.args.get("key") == "project_convention" + + +def test_action_trace_keeps_non_memory_args_verbatim() -> None: + """The redaction is scoped to memory.* — other capabilities are untouched.""" + registry = CapabilityRegistry() + cap = Capability( + capability_id="billing.refund", + name="refund", + description="issue refund", + safety_class=SafetyClass.WRITE, + ) + registry.register(cap) + driver = InMemoryDriver(driver_id="billing") + driver.register_handler("billing.refund", lambda _ctx: [{"ok": True}]) + router = StaticRouter(routes={"billing.refund": ["billing"]}) + kernel = Kernel( + registry=registry, + token_provider=HMACTokenProvider(secret="test-secret-do-not-use-in-prod"), + router=router, + ) + kernel.register_driver(driver) + principal = Principal(principal_id="agent-1", roles=["writer"]) + request = CapabilityRequest( + capability_id="billing.refund", goal="user requested refund processing" + ) + grant = kernel.grant_capability( + request, principal, justification="refund authorized in ticket #12345" + ) + asyncio.run( + kernel.invoke( + grant.token, + principal=principal, + args={"payload": "this is not memory data", "amount": 42}, + ) + ) + trace = kernel._trace_store.list_all()[-1] + assert trace.args.get("payload") == "this is not memory data" + assert trace.args.get("amount") == 42 diff --git a/tests/test_handles.py b/tests/test_handles.py index 12689ae..909b2ec 100644 --- a/tests/test_handles.py +++ b/tests/test_handles.py @@ -6,8 +6,14 @@ import pytest -from agent_kernel import HandleExpired, HandleNotFound, HandleStore +from agent_kernel import ( + HandleConstraintViolation, + HandleExpired, + HandleNotFound, + HandleStore, +) from agent_kernel.models import Handle +from agent_kernel.policy_reasons import DenialReason @pytest.fixture() @@ -145,3 +151,170 @@ def test_periodic_eviction_on_store() -> None: s.store("cap.x", [i], ttl_seconds=-1) # All expired entries should have been evicted at the interval boundary assert len(s._meta) == 0 + + +# ── Grant-constraint expand (#76) ───────────────────────────────────────────── + + +def _granted_rows() -> list[dict[str, object]]: + return [ + {"id": i, "email": f"u{i}@example.com", "status": "ok", "region": "eu" if i < 10 else "us"} + for i in range(20) + ] + + +def test_store_persists_grant_constraints(store: HandleStore) -> None: + handle = store.store( + "cap.x", + _granted_rows(), + principal_id="p-1", + constraints={"max_rows": 5, "allowed_fields": ["id", "status"]}, + ) + assert handle.principal_id == "p-1" + assert handle.constraints == {"max_rows": 5, "allowed_fields": ["id", "status"]} + + +def test_expand_denies_limit_over_granted_max_rows(store: HandleStore) -> None: + handle = store.store("cap.x", _granted_rows(), constraints={"max_rows": 3}) + with pytest.raises(HandleConstraintViolation) as exc: + store.expand(handle, query={"limit": 50}) + assert exc.value.reason_code == DenialReason.HANDLE_CONSTRAINT_VIOLATION + + +def test_expand_caps_unspecified_limit_to_grant_max_rows(store: HandleStore) -> None: + handle = store.store("cap.x", _granted_rows(), constraints={"max_rows": 3}) + frame = store.expand(handle, query={}) + # 20 source rows, but grant caps at 3. + assert len(frame.table_preview) == 3 + + +def test_expand_denies_disallowed_fields(store: HandleStore) -> None: + handle = store.store( + "cap.x", + _granted_rows(), + constraints={"allowed_fields": ["id", "status"]}, + ) + with pytest.raises(HandleConstraintViolation) as exc: + store.expand(handle, query={"fields": ["id", "email"]}) + assert exc.value.reason_code == DenialReason.HANDLE_CONSTRAINT_VIOLATION + assert "email" in str(exc.value) + + +def test_expand_applies_allowed_fields_when_none_requested(store: HandleStore) -> None: + handle = store.store( + "cap.x", + _granted_rows(), + constraints={"allowed_fields": ["id", "status"]}, + ) + frame = store.expand(handle, query={}) + assert frame.table_preview, "table preview must not be empty" + for row in frame.table_preview: + assert set(row.keys()) == {"id", "status"}, ( + "disallowed grant field leaked through unscoped expand" + ) + + +def test_expand_scope_enforces_filter_dimension(store: HandleStore) -> None: + handle = store.store( + "cap.x", + _granted_rows(), + constraints={"scope": {"region": "eu"}}, + ) + # Asking for region=us on an eu-scoped grant must be denied. + with pytest.raises(HandleConstraintViolation) as exc: + store.expand(handle, query={"filter": {"region": "us"}}) + assert exc.value.reason_code == DenialReason.HANDLE_CONSTRAINT_VIOLATION + + +def test_expand_scope_filter_is_default_and_blocks_us_rows(store: HandleStore) -> None: + handle = store.store( + "cap.x", + _granted_rows(), + constraints={"scope": {"region": "eu"}}, + ) + frame = store.expand(handle, query={}) + # All 20 rows pre-filter; only the eu half should pass through. + assert frame.table_preview, "scoped expand must return at least one row" + for row in frame.table_preview: + assert row["region"] == "eu", "us-region row leaked through scoped expand" + + +def test_expand_principal_mismatch_denied(store: HandleStore) -> None: + handle = store.store( + "cap.x", + _granted_rows(), + principal_id="p-original", + ) + with pytest.raises(HandleConstraintViolation) as exc: + store.expand(handle, query={}, principal_id="p-attacker") + assert exc.value.reason_code == DenialReason.HANDLE_PRINCIPAL_MISMATCH + + +def test_expand_principal_missing_denied_for_bound_handle(store: HandleStore) -> None: + """A principal-bound handle cannot be expanded without proving the principal. + + Regression: an earlier implementation only enforced the binding when both + sides were non-empty, so a caller could bypass by passing principal_id="" + (or by omitting it via the default). + """ + handle = store.store("cap.x", _granted_rows(), principal_id="p-original") + # No principal_id argument at all. + with pytest.raises(HandleConstraintViolation) as exc: + store.expand(handle, query={}) + assert exc.value.reason_code == DenialReason.HANDLE_PRINCIPAL_MISMATCH + # Explicit empty string. + with pytest.raises(HandleConstraintViolation) as exc2: + store.expand(handle, query={}, principal_id="") + assert exc2.value.reason_code == DenialReason.HANDLE_PRINCIPAL_MISMATCH + + +def test_expand_principal_same_succeeds(store: HandleStore) -> None: + handle = store.store("cap.x", _granted_rows(), principal_id="p-1") + frame = store.expand(handle, query={"limit": 5}, principal_id="p-1") + assert len(frame.table_preview) == 5 + + +def test_expand_unbound_handle_still_works_without_principal(store: HandleStore) -> None: + """Handles without a stored principal_id are not principal-bound — callers + can still expand them without supplying a principal_id (back-compat).""" + handle = store.store("cap.x", _granted_rows()) # no principal_id + frame = store.expand(handle, query={"limit": 3}) + assert len(frame.table_preview) == 3 + + +# ── Query input validation (#75 / #76 review feedback) ───────────────────────── + + +def test_expand_invalid_filter_type_raises_stable(store: HandleStore) -> None: + handle = store.store("cap.x", _granted_rows()) + with pytest.raises(HandleConstraintViolation) as exc: + store.expand(handle, query={"filter": ["not-a-dict"]}) + assert exc.value.reason_code == DenialReason.INVALID_CONSTRAINT + + +def test_expand_invalid_fields_type_raises_stable(store: HandleStore) -> None: + handle = store.store("cap.x", _granted_rows()) + with pytest.raises(HandleConstraintViolation) as exc: + store.expand(handle, query={"fields": "id"}) # string, not list + assert exc.value.reason_code == DenialReason.INVALID_CONSTRAINT + + +def test_expand_invalid_offset_raises_stable(store: HandleStore) -> None: + handle = store.store("cap.x", _granted_rows()) + with pytest.raises(HandleConstraintViolation) as exc: + store.expand(handle, query={"offset": "abc"}) + assert exc.value.reason_code == DenialReason.INVALID_CONSTRAINT + + +def test_expand_invalid_limit_raises_stable(store: HandleStore) -> None: + handle = store.store("cap.x", _granted_rows()) + with pytest.raises(HandleConstraintViolation) as exc: + store.expand(handle, query={"limit": "abc"}) + assert exc.value.reason_code == DenialReason.INVALID_CONSTRAINT + + +def test_expand_no_constraints_is_unchanged(store: HandleStore) -> None: + # Handles created without constraints still behave like the legacy code path. + handle = store.store("cap.x", _granted_rows()) + frame = store.expand(handle, query={"limit": 2}) + assert len(frame.table_preview) == 2 diff --git a/tests/test_kernel.py b/tests/test_kernel.py index d99e154..115362f 100644 --- a/tests/test_kernel.py +++ b/tests/test_kernel.py @@ -49,7 +49,9 @@ async def test_full_flow(kernel: Kernel, reader_principal: Principal) -> None: # expand assert frame.handle is not None - expanded = kernel.expand(frame.handle, query={"offset": 0, "limit": 2}) + expanded = kernel.expand( + frame.handle, query={"offset": 0, "limit": 2}, principal=reader_principal + ) assert len(expanded.table_preview) <= 2 diff --git a/tests/test_logging.py b/tests/test_logging.py index b415325..b0c1712 100644 --- a/tests/test_logging.py +++ b/tests/test_logging.py @@ -140,7 +140,7 @@ async def test_no_secret_material_in_logs( args={}, ) assert frame.handle is not None - log_kernel.expand(frame.handle, query={"offset": 0, "limit": 1}) + log_kernel.expand(frame.handle, query={"offset": 0, "limit": 1}, principal=reader) log_kernel.explain(frame.action_id) all_text = _all_log_text(caplog.records) @@ -216,7 +216,7 @@ async def test_expand_emits_info( assert frame.handle is not None with caplog.at_level(logging.INFO, logger="agent_kernel.kernel"): - log_kernel.expand(frame.handle, query={}) + log_kernel.expand(frame.handle, query={}, principal=reader) assert any( "expand" in r.getMessage() for r in caplog.records if r.name == "agent_kernel.kernel" diff --git a/tests/test_policy.py b/tests/test_policy.py index f4125d1..977380e 100644 --- a/tests/test_policy.py +++ b/tests/test_policy.py @@ -272,6 +272,117 @@ def test_secrets_denied_writer_role(engine: DefaultPolicyEngine) -> None: engine.evaluate(_req("cap.sec"), cap, p, justification="long enough justification here") +# ── MEMORY (#75) ────────────────────────────────────────────────────────────── + + +def _memory_request(scope: dict[str, object] | None = None) -> CapabilityRequest: + return CapabilityRequest( + capability_id="memory.read", + goal="test memory", + scope=scope or {}, + ) + + +def test_memory_read_project_scope_allowed(engine: DefaultPolicyEngine) -> None: + """Reading project-scoped memory works for any reader.""" + p = Principal(principal_id="u1", roles=["reader"]) + cap = _cap("memory.read", SafetyClass.READ, SensitivityTag.MEMORY) + dec = engine.evaluate(_memory_request({"memory_scope": "project"}), cap, p, justification="") + assert dec.allowed is True + + +def test_memory_read_sensitive_denied_without_role(engine: DefaultPolicyEngine) -> None: + """Reading sensitive-scoped memory requires memory_reader_sensitive.""" + from agent_kernel.policy_reasons import DenialReason + + p = Principal(principal_id="u1", roles=["reader"]) + cap = _cap("memory.read", SafetyClass.READ, SensitivityTag.MEMORY) + with pytest.raises(PolicyDenied) as exc: + engine.evaluate(_memory_request({"memory_scope": "sensitive"}), cap, p, justification="") + assert exc.value.reason_code == DenialReason.MEMORY_SENSITIVE_READ_DENIED + + +def test_memory_read_sensitive_allowed_with_role(engine: DefaultPolicyEngine) -> None: + p = Principal(principal_id="u1", roles=["reader", "memory_reader_sensitive"]) + cap = _cap("memory.read", SafetyClass.READ, SensitivityTag.MEMORY) + dec = engine.evaluate(_memory_request({"memory_scope": "sensitive"}), cap, p, justification="") + assert dec.allowed is True + + +def test_memory_read_sensitive_allowed_with_admin(engine: DefaultPolicyEngine) -> None: + p = Principal(principal_id="u1", roles=["admin"]) + cap = _cap("memory.read", SafetyClass.READ, SensitivityTag.MEMORY) + dec = engine.evaluate(_memory_request({"memory_scope": "sensitive"}), cap, p, justification="") + assert dec.allowed is True + + +def test_memory_write_denied_without_writer_role(engine: DefaultPolicyEngine) -> None: + """memory.write requires the memory_writer role even when SafetyClass=WRITE + would otherwise be satisfied by a generic writer.""" + from agent_kernel.policy_reasons import DenialReason + + p = Principal(principal_id="u1", roles=["writer"]) + cap = _cap("memory.write", SafetyClass.WRITE, SensitivityTag.MEMORY) + req = CapabilityRequest(capability_id="memory.write", goal="store a note") + with pytest.raises(PolicyDenied) as exc: + engine.evaluate(req, cap, p, justification="needs durable notes for session") + # Writer role passes the safety_class check (no MISSING_ROLE there) and we + # reach the MEMORY branch. + assert exc.value.reason_code == DenialReason.MEMORY_WRITE_REQUIRES_WRITER + + +def test_memory_write_allowed_with_memory_writer_role(engine: DefaultPolicyEngine) -> None: + p = Principal(principal_id="u1", roles=["writer", "memory_writer"]) + cap = _cap("memory.write", SafetyClass.WRITE, SensitivityTag.MEMORY) + req = CapabilityRequest(capability_id="memory.write", goal="store a note") + dec = engine.evaluate(req, cap, p, justification="needs durable notes for session") + assert dec.allowed is True + + +def test_memory_write_allowed_with_admin(engine: DefaultPolicyEngine) -> None: + p = Principal(principal_id="u1", roles=["admin"]) + cap = _cap("memory.write", SafetyClass.WRITE, SensitivityTag.MEMORY) + req = CapabilityRequest(capability_id="memory.write", goal="store a note") + dec = engine.evaluate(req, cap, p, justification="needs durable notes for session") + assert dec.allowed is True + + +def test_memory_destructive_requires_writer(engine: DefaultPolicyEngine) -> None: + """DESTRUCTIVE memory (e.g. memory.forget) is treated as write-class.""" + from agent_kernel.policy_reasons import DenialReason + + p = Principal(principal_id="u1", roles=["admin"]) + # admin passes the DESTRUCTIVE safety_class. The MEMORY branch then needs + # memory_writer OR admin; admin satisfies it. + cap = _cap("memory.forget", SafetyClass.DESTRUCTIVE, SensitivityTag.MEMORY) + req = CapabilityRequest(capability_id="memory.forget", goal="purge notes") + dec = engine.evaluate(req, cap, p, justification="user requested deletion of all notes") + assert dec.allowed is True + + # A principal with destructive role but no memory_writer fails the memory rule. + p2 = Principal(principal_id="u2", roles=["writer"]) + cap2 = _cap("memory.forget", SafetyClass.DESTRUCTIVE, SensitivityTag.MEMORY) + with pytest.raises(PolicyDenied) as exc: + engine.evaluate(req, cap2, p2, justification="user requested deletion of all notes") + # writer is not admin, so DESTRUCTIVE fails first on MISSING_ROLE. + assert exc.value.reason_code == DenialReason.MISSING_ROLE + + +def test_memory_explain_lists_failed_conditions() -> None: + """explain() lists the memory denial alongside the FailedCondition reason_code.""" + from agent_kernel.policy_reasons import DenialReason + + eng = DefaultPolicyEngine() + p = Principal(principal_id="u1", roles=["reader"]) + cap = _cap("memory.read", SafetyClass.READ, SensitivityTag.MEMORY) + explanation = eng.explain( + _memory_request({"memory_scope": "sensitive"}), cap, p, justification="" + ) + assert explanation.denied is True + codes = [fc.reason_code for fc in explanation.failed_conditions] + assert str(DenialReason.MEMORY_SENSITIVE_READ_DENIED) in codes + + # ── Confused-deputy binding (via token) ────────────────────────────────────────