Skip to content

feat: enforce live agent credential admission - #6068

Merged
danielkov merged 12 commits into
mainfrom
daniel/aim-193-feat-enforce-live-agent-credential-admission
Sep 9, 2026
Merged

danielkov merged 12 commits into
mainfrom
daniel/aim-193-feat-enforce-live-agent-credential-admission

Conversation

@danielkov

@danielkov danielkov commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Enforces fail-closed live admission for principal-backed agent credentials by combining immutable delegated policy with the current direct agent and owner policies for every authorization check. It preserves canonical agent actor attribution in authorization telemetry, challenges, and audit writes while retaining credential, authorizer, and current-owner provenance separately.

Impact

Principal-backed API keys now deny malformed, expired, mixed, unsupported, inactive-parent, owner-ineligible, and policy-unsatisfied requests. Existing legacy API keys and human-backed transports retain their prior behavior; principal keys are admitted only on producer and consumer transport routes.

Request and MCP telemetry record the trusted principal actor separately from human identity, owner, and credential authorizer. Durable deployment and skill creator fields remain human-only; extending them to reference the generic principal is explicitly deferred to M3 — Principal-based creator identity in the Linear project (AIM-193). This deferral does not add write restrictions or relax existing human-only guards, and does not block this stack.

Technical details

Three-policy admission

A versioned delegated-policy codec validates canonical requested/effective grants and evaluates only effective grants. Admission resolves agent lifecycle, ownership, owner eligibility, direct agent policy, and current-owner policy in one read-only repeatable-read snapshot, then evaluates immutable R, live A, and live O independently for each concrete check.

Fail-closed attribution and bypass handling

Principal-backed contexts force authorization enforcement, reject explicit loaded-grant bypasses and legacy-only routes, and record agent:<uuid> as the canonical actor in authorization telemetry, challenges, and audit writes. Unknown or retired policy entries are ignored per entry, while malformed or noncanonical profiles deny as a whole. Agent policy writes already reject deny effects and unsafe scopes rather than silently accepting unsupported exclusions.

Dependency base

This PR targets main. The authorization and credential-schema prerequisites are merged as #6055 and #6039. API-key lifecycle follows in #6071, and MCP session issuance, access, and refresh follow in #6072.

@danielkov
danielkov requested a review from a team as a code owner September 4, 2026 09:09
@linear-code

linear-code Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

AIM-193

@changeset-bot

changeset-bot Bot commented Sep 4, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: e87e368

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
server Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Running ultrareview automatically — This change enforces fail-closed admission for principal-backed credentials across auth, authorization, and audit paths, and any subtle bug could break access control or allow unauthorized access, so it warrants an ultrareview.. I'll post findings when complete.

@blacksmith-sh

This comment has been minimized.

@danielkov
danielkov force-pushed the daniel/aim-193-feat-enforce-live-agent-credential-admission branch from 98e4146 to 1c4eb02 Compare September 4, 2026 09:19

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ultrareview completed in 18m 33s

All reported issues were addressed

Tip: instead of fixing issues one by one fix them all with cubic

Re-trigger cubic

Comment thread server/internal/auth/key.go
Comment thread server/internal/auth/key.go
Comment thread server/internal/authz/challenge_logger.go
Comment thread server/internal/auth/key.go
Comment thread server/internal/contextvalues/context.go
Comment thread server/internal/agents/runtimepolicy/delegated_policy.go
Comment thread server/internal/auth/key_test.go
@blacksmith-sh

This comment has been minimized.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All reported issues were addressed across 14 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Fix all with cubic | Re-trigger cubic

Comment thread server/internal/chat/impl.go Outdated
@danielkov
danielkov requested a review from a team as a code owner September 4, 2026 09:58

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All reported issues were addressed across 4 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Fix all with cubic | Re-trigger cubic

Comment thread server/internal/chat/impl.go Outdated
@danielkov
danielkov force-pushed the integration/aim-193-prerequisites branch from 3cd345e to 972317d Compare September 8, 2026 20:51
@danielkov
danielkov force-pushed the daniel/aim-193-feat-enforce-live-agent-credential-admission branch from cdb5b55 to 8894d76 Compare September 8, 2026 20:51
@blacksmith-sh

This comment has been minimized.

@danielkov
danielkov force-pushed the daniel/aim-193-feat-enforce-live-agent-credential-admission branch from 8894d76 to e64714b Compare September 8, 2026 21:28
@danielkov
danielkov force-pushed the integration/aim-193-prerequisites branch from 972317d to db91a5a Compare September 8, 2026 21:28
@danielkov
danielkov force-pushed the daniel/aim-193-feat-enforce-live-agent-credential-admission branch 2 times, most recently from 4ac182e to 563d620 Compare September 8, 2026 21:38
@danielkov
danielkov force-pushed the integration/aim-193-prerequisites branch from fec3154 to 1f0a170 Compare September 8, 2026 21:38
@danielkov
danielkov force-pushed the daniel/aim-193-feat-enforce-live-agent-credential-admission branch from 563d620 to e4cc4a4 Compare September 8, 2026 22:17
@danielkov
danielkov changed the base branch from integration/aim-193-prerequisites to main September 8, 2026 22:17
@daviddanialy

Copy link
Copy Markdown
Contributor

deny grants on an agent principal are silently dropped. LoadAgentPolicy skips any scope that fails ValidateRuntimeScope, and every *:blocked_* scope is registered with safeSince: 0, so an exception written on agent:<id> never lands in A. an admin who puts project:blocked_read on an agent will think it's carved out and it won't be. only the owner's exclusions in O actually bite.

either keep blocked scopes in A or reject them at write time for agent principals.

runtimepolicy/load.go:35, runtimepolicy/scopes.go:66

@daviddanialy

Copy link
Copy Markdown
Contributor

private mcp now runs admission twice per request. mcp/impl.go:1061 and serveendpoint.go:797 call PrepareContext after auth already did, and the comment there says it's a no op if grants are loaded, but for principal keys it reruns the whole thing. that's two repeatable read txs at ~9 queries each on the tool call path, with no caching by design. LoadAgentPolicy also refetches the agent row ResolvePrincipal just loaded.

side effect: a revoke that lands between the two calls comes back as a logged 500 instead of a 401.

@daviddanialy

Copy link
Copy Markdown
Contributor

principal keys get UserID = "" in key.go:338. audit rows are rescued by applyAuthenticatedActor but nothing else falls back to the agent actor. deployments.user_id gets stored as an empty string, telemetry user_id is empty, and mcp/audit_assistant.go:47 skips the audit write entirely when the user id is blank, so agent tool calls through that path aren't audited at all. several other handlers early return on an empty user id too.

fine to punt to a follow up since nothing mints these keys yet, but worth noting in the pr.

@daviddanialy

Copy link
Copy Markdown
Contributor

description is stale. the logout redirect, clear site data, 8s race and agent management flag bullets are all from #6042 / #6055 which are already on main, and the note about targeting the integration branch is out of date. also no changeset for the server bump.

@daviddanialy

Copy link
Copy Markdown
Contributor

approved pending comment handling

@danielkov

Copy link
Copy Markdown
Contributor Author

Verified the general review comments against the current head:

  • Deny grants (comment): both agent policy create/update already call validatePolicyGrant, which rejects non-allow effects and unsafe scopes. No additional write-path fix needed.
  • Attribution (comment): documented the accepted principal-aware attribution follow-up before issuance rollout in the PR impact section. The assistant audit path requires an assistant principal; principal API keys do not establish one.
  • Description (comment): removed the stale integration-base summary and clarified this PR's scope.

The duplicate-admission 500 mapping is valid and is being fixed with regression coverage; retaining live re-admission rather than introducing a cache.

@danielkov

Copy link
Copy Markdown
Contributor Author

Fixed the private MCP admission error mapping from this comment in 05e385b. Both callers preserve expected authentication denials and continue logging unexpected failures. Six focused regression cases pass; the denial cases were verified to fail before the fix. The standard package run was blocked by ClickHouse readiness before tests executed.

Attribution is now being addressed in this PR rather than treated solely as a follow-up. Merge remains on hold.

@danielkov

Copy link
Copy Markdown
Contributor Author

Request/MCP telemetry attribution and the server changeset are addressed in 0263442. Server-generated telemetry now carries the trusted agent actor separately from credential, authorizer, owner, and human identity; conflicting caller-supplied actor properties cannot override it. Bulk ingestion is unchanged. Focused attribution tests and commit hooks pass; infrastructure-dependent package runs remain blocked locally by ClickHouse readiness.

Deployment and skill resource creator fields still have human-only contracts. Merge is held pending the choice between explicit rejection of unsupported principal writes and separate principal-creator schema/API support. This is not yet a complete resolution of resource creator attribution.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All reported issues were addressed across 13 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Fix all with cubic | Re-trigger cubic

Comment thread server/internal/auth/auth.go Outdated
@danielkov
danielkov added this pull request to the merge queue Sep 9, 2026
Merged via the queue into main with commit 7628708 Sep 9, 2026
46 checks passed
@danielkov
danielkov deleted the daniel/aim-193-feat-enforce-live-agent-credential-admission branch September 9, 2026 13:59
@github-actions github-actions Bot locked and limited conversation to collaborators Sep 9, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants