feat: hierarchical agent credentials and per-intent CLI selection - #405
feat: hierarchical agent credentials and per-intent CLI selection#405JWThewes wants to merge 4 commits into
Conversation
| } | ||
| if (binding) bindings = [binding]; | ||
| } else { | ||
| const requestedCli = payload.requestedCli || meta?.agentCli || null; |
There was a problem hiding this comment.
for discussion-assist-start on a DRAFT intent, payload does not contain requestedCli and meta.agentCli is still null. Therefore, no credentials are resolved, availableClis is empty, and the assistant fails. Could we pass and resolve the user’s selected CLI and credential binding for this command, and add a test for this scenario?
|
Please also update documentation (at least |
| /> | ||
| } | ||
| description="Which AI agent CLI runs this space's work — only CLIs installed in the deployment are selectable." | ||
| description="Highlighted when a user chooses a CLI for an intent; each user still makes an explicit selection." |
There was a problem hiding this comment.
strange description, looks super generic and not targeting the user, more like a comment
| import { SecretField } from '@/components/settings/SecretField'; | ||
| import { SaveStatusButton, type SaveResult } from '@/components/settings/SaveStatusButton'; | ||
|
|
||
| type Scope = 'platform' | 'space' | 'personal'; |
There was a problem hiding this comment.
Wondering why we don't have the intent scope. As of now the intent binds to the selected scope, but if we want to create a dedicated api key for one intent ?
| const ATTACHMENT_INGEST_POLL_MS = 500; | ||
| const ATTACHMENT_INGEST_TIMEOUT_MS = 30_000; | ||
| const AGENT_CLIS: AgentCli[] = ['kiro', 'claude', 'opencode', 'codex']; | ||
| const AGENT_CLI_LABELS: Record<AgentCli, string> = { |
There was a problem hiding this comment.
We have this config at different places in the frontend:
AgentTab.tsx, DefaultModelsCard.tsx, TierModelsSection.tsx and now also IntentView.tsx plus this one.
Could we centralize this in one place ?
| // Backward-compatible helper retained for focused tests and local tooling. It | ||
| // resolves the legacy platform paths into the supplied object, but production | ||
| // AgentCore no longer calls it or mutates process.env. | ||
| export const resolveAgentAuth = async ({ env = {}, getParam } = {}) => { |
There was a problem hiding this comment.
This function is not used anywhere apart from the tests.
|
|
||
| // Only commands that can spawn an agent CLI need hierarchical credential | ||
| // resolution. Engine-only commands must remain available during an SSM outage. | ||
| const AGENT_AUTH_COMMANDS = new Set([ |
There was a problem hiding this comment.
We could probably simplify / merge this with the handler definition line 110
There are also some if in the resolveInvocationAgentAuth() function that could benefit from this refactor, as we have several place where we check those strings.
const COMMANDS = {
'init-ws': {
handler: 'initWs',
agentAuth: false,
},
'run-stage': {
handler: 'runStage',
agentAuth: true,
},
// ...
| platform: 'Platform', | ||
| } as const; | ||
|
|
||
| const credentialFailureMessage = (failureReason: string | null) => { |
There was a problem hiding this comment.
A bit fragile, if the wording changes in the backend, it will silently fail. Maybe the API should return a proper structure (code +message)
| // A session id >= 33 chars is required by InvokeAgentRuntime; the capabilities | ||
| // command is stateless so any stable id works. | ||
| const CAPABILITIES_SESSION_ID = 'aidlc-capabilities-probe-00000001'; | ||
| const PLATFORM_CREDENTIAL_BINDINGS = { |
There was a problem hiding this comment.
in shared/agent-credentials.js we already define a set of constants that could be reused here to avoid creating a copy of the mapping
| "arn:${local.partition}:ssm:${data.aws_region.current.region}:${data.aws_caller_identity.current.account_id}:parameter/${var.project_name}/${var.environment}/users/*/agent-credentials/*", | ||
| "arn:${local.partition}:ssm:${data.aws_region.current.region}:${data.aws_caller_identity.current.account_id}:parameter/${var.project_name}/${var.environment}/projects/*/agent-credentials/*", |
There was a problem hiding this comment.
This resource is correctly limited to the current application and environment, but the wildcards still allow the shared AgentCore role to decrypt every user and space credential in that deployment. AgentCore also has permission to scan execution metadata containing credential bindings, so these paths are discoverable. Could credential lookup be moved behind the existing broker pattern, so AgentCore can request only the binding authorized for the current invocation? (AgentCore -> agent-credential broker -> SSM)
|
@jeromevdl addressed your comments |
| missingCredentialBindings.push(credentialBinding); | ||
| continue; | ||
| } | ||
| invocationEnv[credentialEnvName(binding.provider)] = value; |
There was a problem hiding this comment.
This PR necessarily passes credentials through the CLI environment. Output redaction should be handled separately and consistently for both agent credentials and MCP environment variables. I’ll track application-level redaction and CloudWatch data-protection policies in a follow-up issue: #406
Not sure everything was addressed... Please mark resolved the comments you have fixed. |
2 participants
Summary
user > space > platformprecedenceTesting
Closes #404