fix(schema): relax request header allowlist to accept broader X-Amzn-Bedrock-AgentCore-* family#1163
fix(schema): relax request header allowlist to accept broader X-Amzn-Bedrock-AgentCore-* family#1163aidandaly24 wants to merge 7 commits into
Conversation
github-actions
Bot
added
size/s
agentcore-cli-automation
left a comment
agentcore-cli-automation
left a comment
There was a problem hiding this comment.
Thanks for pushing this forward. I think the fix is on the right track but is under-relaxing the validation relative to what the linked AWS documentation now describes. See the inline comment on agent-env.ts for details — I think that needs to be resolved before merging. The other note is a question/clarification rather than a blocker.
| */ | ||
| export const HEADER_ALLOWLIST_PREFIX = 'X-Amzn-Bedrock-AgentCore-Runtime-Custom-'; | ||
| export const HEADER_ALLOWLIST_BROAD_PREFIX = 'X-Amzn-Bedrock-AgentCore-'; | ||
| export const HEADER_ALLOWLIST_PATTERN = /^(Authorization|X-Amzn-Bedrock-AgentCore-[A-Za-z0-9-]+)$/; |
There was a problem hiding this comment.
This fix (and, by extension, the mirror change in agentcore-l3-cdk-constructs) only relaxes the allowlist to X-Amzn-Bedrock-AgentCore-*, but the AWS doc linked in the PR description and in issue #1151 actually describes a much broader rule:
You can pass any valid HTTP header that is not in the restricted headers list, including webhook signatures like
X-Custom-Signature, API keys likeX-Api-Key, trace context, or session identifiers.
The doc's own example requestHeaderAllowlist is:
"requestHeaderAllowlist": [
"X-Custom-Signature",
"X-Api-Key",
"X-Amzn-Bedrock-AgentCore-Runtime-Custom-UserId"
]With the regex in this PR (/^(Authorization|X-Amzn-Bedrock-AgentCore-[A-Za-z0-9-]+)$/), a user who copy/pastes that exact example from the AWS docs into agentcore.json will still fail Zod validation on X-Custom-Signature and X-Api-Key. That seems like the core bug the issue is asking us to fix, and it isn't addressed here.
A few possible options:
-
Match the doc rule directly. Accept any header name matching a valid HTTP header token (
[A-Za-z0-9_-]+or RFC 7230 tchar) except:- names starting with
x-amz-(case-insensitive) - names starting with
x-amzn-(case-insensitive) except those starting withX-Amzn-Bedrock-AgentCore-Runtime-Custom- - optionally, the explicit restricted-header list from the doc (Host, Authorization-adjacent, Cookie, etc.) — though that list is large and may be better left to the service to reject.
- names starting with
-
Accept any valid HTTP header token plus
Authorization, and leave it to the service to reject restricted names. This is the simplest and keeps us from having to chase the restricted list as it evolves. It would still be a strict improvement over today. -
Keep this PR as a narrow, intentional fix for only the
X-Amzn-Bedrock-AgentCore-*family and explicitly scope the issue (and issue Relax Custom Header regex to allow new pattern #1151) to that — but in that case the PR description and the.refine()error message should be updated to reflect that headers likeX-Api-Key/X-Custom-Signatureare knowingly not yet supported here, and the issue should stay open for follow-up.
Related: the HEADER_ALLOWLIST_PATTERN is exported and also used by normalizeHeaderName, which still unconditionally prepends X-Amzn-Bedrock-AgentCore-Runtime-Custom- to any header that doesn't already start with X-Amzn-Bedrock-AgentCore-. That existing behavior means agentcore invoke -H "X-Api-Key: foo" silently rewrites to X-Amzn-Bedrock-AgentCore-Runtime-Custom-X-Api-Key. Whichever option above you pick, normalizeHeaderName probably needs a matching adjustment (or at minimum a comment) so the two stay consistent.
| .refine( | ||
| val => val === 'Authorization' || val.startsWith(HEADER_ALLOWLIST_PREFIX), | ||
| `Must be "Authorization" or start with "${HEADER_ALLOWLIST_PREFIX}"` | ||
| val => HEADER_ALLOWLIST_PATTERN.test(val), |
There was a problem hiding this comment.
Minor behavioral tightening to confirm is intentional: the old predicate was val === 'Authorization' || val.startsWith(HEADER_ALLOWLIST_PREFIX), which would accept a header that is exactly the prefix (empty suffix, e.g. X-Amzn-Bedrock-AgentCore-Runtime-Custom-). The new regex requires [A-Za-z0-9-]+ after the broad prefix, so bare-prefix strings now fail validation. That's almost certainly the right call, but worth calling out explicitly since it's a (small) validation tightening on top of the relaxation.
| if (input.toLowerCase().startsWith(HEADER_ALLOWLIST_PREFIX.toLowerCase())) { | ||
| return `${HEADER_ALLOWLIST_PREFIX}${input.slice(HEADER_ALLOWLIST_PREFIX.length)}`; | ||
| } | ||
| if (input.toLowerCase().startsWith(HEADER_ALLOWLIST_BROAD_PREFIX.toLowerCase())) { |
There was a problem hiding this comment.
Nit on the re-export: HEADER_ALLOWLIST_PATTERN is imported and re-exported here but it isn't used anywhere in header-utils.ts (the two startsWith checks use the two string prefix constants instead). If nothing else in the CLI consumes HEADER_ALLOWLIST_PATTERN via this module, the re-export can be dropped; otherwise no action needed.
github-actions
Bot
removed
the
agentcore-harness-reviewing
Coverage Report
|
github-actions
Bot
added
size/s
agentcore-cli-automation
left a comment
agentcore-cli-automation
left a comment
There was a problem hiding this comment.
Thanks for iterating on the earlier feedback — the broadened RFC 7230 regex in the schema now matches the AWS docs (e.g. X-Api-Key, X-Custom-Signature), and the pass-through branch in normalizeHeaderName cleanly avoids silently rewriting non-AgentCore X-* headers. The test additions are on point.
Two issues worth addressing before this merges — details in the inline comments. Neither is a logic bug in this repo, but together they would leave users with a confusing end-to-end experience.
| .refine( | ||
| val => val === 'Authorization' || val.startsWith(HEADER_ALLOWLIST_PREFIX), | ||
| `Must be "Authorization" or start with "${HEADER_ALLOWLIST_PREFIX}"` | ||
| val => HEADER_ALLOWLIST_PATTERN.test(val), |
There was a problem hiding this comment.
Merge-order / downstream compatibility. The PR description notes that a mirror change is being made in agentcore-l3-cdk-constructs, but the current published @aws/agentcore-cdk (pinned to ^0.1.0-alpha.19 in src/assets/cdk/package.json) still carries the strict val === 'Authorization' || val.startsWith(HEADER_ALLOWLIST_PREFIX) predicate.
That means once this ships, agentcore.json validation will happily accept e.g. "X-Api-Key" in the CLI, but when the user runs agentcore deploy / the synthesized CDK app parses the same config, the bundled L3 construct's Zod schema will reject it with the old error message — and the error will appear to come from CDK synth, not from the CLI that just validated the field.
A few ways to resolve:
- Land and publish the mirror change in
agentcore-l3-cdk-constructsfirst, then bump the@aws/agentcore-cdkpin insrc/assets/cdk/package.jsonin this PR (or a follow-up that blocks the CLI release). - Hold this PR until the constructs release is out, then merge together.
- If you want to land this now, at minimum add a note in the PR / release notes that users on older
@aws/agentcore-cdkversions will still see the old restriction at deploy time.
Option 1 or 2 are strongly preferable — otherwise the "fix" only moves the failure downstream instead of eliminating it.
| // Any other header that already looks like a full HTTP header name (starts with "X-") | ||
| // is passed through unchanged — do not silently rewrite e.g. "X-Api-Key" into a | ||
| // Custom- prefixed name. | ||
| if (/^x-/i.test(input)) { |
There was a problem hiding this comment.
Stale user-facing help strings. The schema and normalizeHeaderName now accept a much broader set of headers than just X-Amzn-Bedrock-AgentCore-Runtime-Custom-*, but the flag help text and the TUI hints still tell users the allowlist is only for custom-prefixed names:
src/cli/primitives/AgentPrimitive.tsx:257—'Comma-separated list of custom header names to allow (auto-prefixed with X-Amzn-Bedrock-AgentCore-Runtime-Custom-) [non-interactive]'src/cli/tui/screens/generate/GenerateWizardUI.tsx:310—Enter header suffixes or full names. We auto-prefix with X-Amzn-Bedrock-AgentCore-Runtime-Custom- if needed. 'Authorization' is also accepted.src/cli/tui/screens/agent/AddAgentScreen.tsx:1184— same copy as the wizard.
After this change, a user typing X-Api-Key or X-Custom-Signature into that field is now valid (per the AWS docs and per normalizeHeaderName's new /^x-/i pass-through branch) but the help text actively suggests it isn't. That's a discoverability regression for the feature this PR is supposed to enable.
Please update those three copies to reflect the new behavior (e.g. something like: Comma-separated list of header names to allow-list (e.g. Authorization, X-Api-Key, X-Amzn-Bedrock-AgentCore-Runtime-Custom-MyHeader). Bare suffixes are auto-prefixed with X-Amzn-Bedrock-AgentCore-Runtime-Custom- for backward compatibility.) so that what the CLI validates and what the CLI advertises stay in sync.
github-actions
Bot
removed
the
agentcore-harness-reviewing
github-actions
Bot
added
size/s
agentcore-cli-automation
left a comment
agentcore-cli-automation
left a comment
There was a problem hiding this comment.
Thanks for the follow-ups — the schema broadening, the updated help text, and the normalizeHeaderName pass-through for X-Api-Key / X-Custom-Signature all look good and correctly address the earlier review feedback.
Two blocking issues before this can merge:
-
The pinned
@aws/agentcore-cdkversion does not exist on npm.src/assets/cdk/package.jsonpins^0.1.0-alpha.29, but the latest published version is0.1.0-alpha.28and0.1.0-alpha.29returnsversion not foundfrom the registry. Inspecting the published0.1.0-alpha.28tarball confirms it still carries the old strict predicate (val === 'Authorization' || val.startsWith(HEADER_ALLOWLIST_PREFIX)). I also don't see the mirror change in theagentcore-l3-cdk-constructsrepo's main branch (still atalpha.22with the old schema). As shipped, this PR would make everyagentcore createscaffold fail atnpm install. See the inline comment onsrc/assets/cdk/package.json. -
The
formatCI check is failing onsrc/schema/schemas/agent-env.ts. Needsnpm run format+ a commit. See the inline comment.
Everything else (the schema/normalizer logic, the new tests, the updated help strings in the CLI flag and both TUI screens) looks reasonable to me.
| }, | ||
| "dependencies": { | ||
| "@aws/agentcore-cdk": "^0.1.0-alpha.19", | ||
| "@aws/agentcore-cdk": "^0.1.0-alpha.29", |
There was a problem hiding this comment.
Blocker: this version doesn't exist on npm yet.
The latest published @aws/agentcore-cdk is 0.1.0-alpha.28 — 0.1.0-alpha.29 returns "version not found" from the registry, and the agentcore-l3-cdk-constructs repo's main branch is still at 0.1.0-alpha.22 with the old strict schema (val === 'Authorization' || val.startsWith(HEADER_ALLOWLIST_PREFIX)) — no mirror PR appears to have landed there yet.
If this merges as-is, every agentcore create will produce a CDK scaffold whose npm install fails with No matching version found for @aws/agentcore-cdk@^0.1.0-alpha.29. This is exactly the downstream-compat failure mode flagged in the earlier review — the pin was bumped, but it was bumped to a version that hasn't been cut yet.
A few ways to unblock:
- Land and publish the mirror change in
agentcore-l3-cdk-constructsfirst (cutting at least0.1.0-alpha.29, or whatever alpha version actually includes the new schema), then land this PR. Preferred. - Hold this PR until the constructs package is published, then retarget the pin to whatever version actually contains the mirror change (
alpha.29,alpha.30, etc. — whatever ends up being cut) and re-push. - Revert this line to
^0.1.0-alpha.28(the currently-published version, which still has the strict schema) and accept thatagentcore deploywill continue to reject broad header names until the constructs package is updated. This makes the CLI-schema change useless end-to-end but at least keepsagentcore createworking.
Option 1 or 2 are strongly preferable. Worth also confirming in the PR description which version of @aws/agentcore-cdk is expected to carry the mirror change, so the reviewer / release manager can verify the two are in sync before merge.
| `Must be "Authorization" or start with "${HEADER_ALLOWLIST_PREFIX}"` | ||
| val => HEADER_ALLOWLIST_PATTERN.test(val), | ||
| 'Must be a valid HTTP header name (RFC 7230 token characters)' | ||
| ) |
There was a problem hiding this comment.
Blocker: format CI check is failing.
The format job on the latest commit fails with:
[warn] src/schema/schemas/agent-env.ts
[warn] Code style issues found in 2 files. Run Prettier with --write to fix.
(The other file, .github/harness/prompts/review.md, isn't touched by this PR and is presumably a pre-existing issue on main, but agent-env.ts is yours.)
Please run npm run format and commit the result.
github-actions
Bot
removed
the
agentcore-harness-reviewing
github-actions
Bot
added
size/s
agentcore-cli-automation
left a comment
agentcore-cli-automation
left a comment
There was a problem hiding this comment.
Thanks for the quick turnaround on the pin — that unblocks agentcore create. Two blocking issues still remain, both flagged inline:
-
formatCI is still failing, just on different files now. The edits to the two TUI help strings inAddAgentScreen.tsxandGenerateWizardUI.tsxbroke Prettier formatting (per the CI log on the latest commit). Needs anothernpm run format+ commit. -
The end-to-end gap is still open. Pinning back to
^0.1.0-alpha.28restoresnpm install, but the published0.1.0-alpha.28tarball still ships the old strict predicate (val === 'Authorization' || val.startsWith(HEADER_ALLOWLIST_PREFIX)) — I verified this by pulling the tarball from the registry. Theagentcore-l3-cdk-constructsrepo'smainis also still onalpha.22with the old schema, and no mirror PR appears to have landed. So as shipped, the CLI now tells usersX-Api-Key/X-Custom-Signature/X-Amzn-Bedrock-AgentCore-Runtime-User-*are valid (help text + Zod schema +normalizeHeaderNamepass-through), but the CDK synth step inagentcore deploywill still reject them with the old error message. That's the exact failure mode my previous review flagged — the pin was "fixed" by reverting rather than by bumping, which only trades one broken state for another.
The cleanest path is still: land + publish the mirror change in agentcore-l3-cdk-constructs first, then bump this pin to whatever alpha version ends up carrying it, then merge this PR. See the inline comment on src/assets/cdk/package.json for the specific options.
Everything else in the PR (schema, normalizer, tests, help-text copy) looks good — the logic side of the fix is in the right place.
| }, | ||
| "dependencies": { | ||
| "@aws/agentcore-cdk": "^0.1.0-alpha.19", | ||
| "@aws/agentcore-cdk": "^0.1.0-alpha.28", |
There was a problem hiding this comment.
Blocker (continuation of the previous merge-order comment): end-to-end fix is still broken.
This now pins to ^0.1.0-alpha.28, which is the current latest on npm, so npm install will succeed — good. But I inspected the published @aws/agentcore-cdk@0.1.0-alpha.28 tarball and its dist/schema/schemas/agent-env.js still contains the old strict predicate:
.refine(val => val === 'Authorization' || val.startsWith(exports.HEADER_ALLOWLIST_PREFIX), ...)And agentcore-l3-cdk-constructs main is still at 0.1.0-alpha.22 with the same old schema — no mirror PR has landed or been published.
So with this PR as-is, a user who puts X-Api-Key in agentcore.json will:
- pass CLI-side Zod validation (new schema in this PR) ✅
- have the CLI help text tell them it's valid ✅
agentcore deploy→ CDK synth → the bundled construct's old Zod schema rejects it withMust be "Authorization" or start with "X-Amzn-Bedrock-AgentCore-Runtime-Custom-"❌
That's arguably worse than before, because now the CLI is advertising a feature it can't deliver end-to-end.
Options, in order of preference:
- Land and publish the mirror change in
agentcore-l3-cdk-constructsfirst, bump this pin to the new alpha (e.g.^0.1.0-alpha.29or whatever ends up being cut), and then merge this PR. This is the only option that actually delivers the fix end-to-end. - Hold this PR until the constructs package is published, then update the pin.
- Merge as-is and explicitly scope the fix to CLI-side validation only, with a release note that
agentcore deploystill rejects the broader header set until the constructs package is updated. This is a half-fix but at least doesn't regressagentcore create.
If you go with option 3, please also (a) update the PR description to say that explicitly, and (b) leave issue #1151 open since the user-facing deploy flow isn't actually fixed.
| needed. 'Authorization' is also accepted. | ||
| Enter header names (e.g. Authorization, X-Api-Key, | ||
| X-Amzn-Bedrock-AgentCore-Runtime-Custom-MyHeader). Bare suffixes are auto-prefixed with | ||
| X-Amzn-Bedrock-AgentCore-Runtime-Custom- for backward compatibility. |
There was a problem hiding this comment.
Blocker: format CI is failing on this file (and on GenerateWizardUI.tsx with the same edit). From the CI log on commit 4d6dd29:
[warn] src/cli/tui/screens/agent/AddAgentScreen.tsx
[warn] src/cli/tui/screens/generate/GenerateWizardUI.tsx
[warn] Code style issues found in 3 files. Run Prettier with --write to fix.
The previous format fix only covered agent-env.ts — the subsequent help-text rewrite re-broke formatting on these two TSX files. Please run npm run format again and commit the result.
| needed. 'Authorization' is also accepted. | ||
| Enter header names (e.g. Authorization, X-Api-Key, | ||
| X-Amzn-Bedrock-AgentCore-Runtime-Custom-MyHeader). Bare suffixes are auto-prefixed with | ||
| X-Amzn-Bedrock-AgentCore-Runtime-Custom- for backward compatibility. |
There was a problem hiding this comment.
Same Prettier failure as AddAgentScreen.tsx:1186. Please include this file in the npm run format fix-up commit.
github-actions
Bot
removed
the
agentcore-harness-reviewing
github-actions
Bot
added
size/s
agentcore-cli-automation
left a comment
agentcore-cli-automation
left a comment
There was a problem hiding this comment.
Reviewed the PR in its current state (commit 76b44cc). The scope narrowing to "CLI-only half-fix" (Option 3) and the corresponding PR-description / issue-#1151 bookkeeping all look good, the schema relaxation matches the AWS docs, and the prior reviewer concerns (stale help strings, pin bumped to a non-existent version, HEADER_ALLOWLIST_PATTERN re-export) have been addressed.
Found one new blocker that hasn't been raised yet: the snapshot for src/assets/cdk/package.json wasn't regenerated after bumping the @aws/agentcore-cdk pin, so assets.snapshot.test.ts is currently failing in CI (unit-test (20.x, 1/3)). See the inline comment on src/assets/cdk/package.json.
Note: the format CI failure on this run is caused only by .github/harness/prompts/review.md, which is untouched by this PR and is a pre-existing issue on main — not a blocker for this PR.
| }, | ||
| "dependencies": { | ||
| "@aws/agentcore-cdk": "^0.1.0-alpha.19", | ||
| "@aws/agentcore-cdk": "^0.1.0-alpha.28", |
There was a problem hiding this comment.
Blocker: snapshot test is failing for this file.
The @aws/agentcore-cdk pin was bumped from ^0.1.0-alpha.19 → ^0.1.0-alpha.28 here, but the corresponding snapshot in src/assets/__tests__/__snapshots__/assets.snapshot.test.ts.snap (line 360) still pins ^0.1.0-alpha.19. As a result, unit-test (20.x, 1/3) is currently failing in CI:
× Assets Directory Snapshots > CDK assets > cdk/cdk/package.json should match snapshot
→ Snapshot `Assets Directory Snapshots > CDK assets > cdk/cdk/package.json should match snapshot 1` mismatched
The PR checklist item "If I modified src/assets/, I ran npm run test:update-snapshots and committed the updated snapshots" is also still unchecked — this is exactly the case it's guarding against.
Fix: run npm run test:update-snapshots locally and commit the updated .snap file.
github-actions
Bot
removed
the
agentcore-harness-reviewing
github-actions
Bot
added
size/s
agentcore-cli-automation
left a comment
agentcore-cli-automation
left a comment
There was a problem hiding this comment.
Reviewed the current state of the PR (head c8abffa). The scope narrowing to "CLI-only half-fix" is well-documented in the PR description, the schema relaxation matches the linked AWS docs, the normalizeHeaderName pass-through branch correctly avoids silently rewriting X-Api-Key / X-Custom-Signature, and all previously-raised blockers (help-text staleness, nonexistent CDK pin, format failures, snapshot drift) have been addressed. All CI checks are passing on the latest commit.
One soft concern worth thinking about before merge — see the inline comment on AddAgentScreen.tsx. It's specifically a consequence of the Option-3 scope and is borderline between "acceptable trade-off the PR already documents" and "worth holding until the constructs mirror publishes", so I'll leave the call to you.
| needed. 'Authorization' is also accepted. | ||
| Enter header names (e.g. Authorization, X-Api-Key, X-Amzn-Bedrock-AgentCore-Runtime-Custom-MyHeader). | ||
| Bare suffixes are auto-prefixed with X-Amzn-Bedrock-AgentCore-Runtime-Custom- for backward | ||
| compatibility. |
There was a problem hiding this comment.
Soft concern (not a hard blocker — leaving the call to you): the help text here (and the matching copy in GenerateWizardUI.tsx:310 and the --request-header-allowlist flag description in AgentPrimitive.tsx:257) now explicitly advertises X-Api-Key as a valid example. But per the PR description, the currently-pinned @aws/agentcore-cdk@^0.1.0-alpha.28 still ships the old strict predicate, so a user who follows this hint and writes X-Api-Key into agentcore.json will:
- Pass
agentcore add/ the TUI wizard validation ✅ - Pass
agentcore.jsonZod validation in the CLI ✅ - Fail at
agentcore deploy→ CDK synth time with the old error message:Must be "Authorization" or start with "X-Amzn-Bedrock-AgentCore-Runtime-Custom-"❌
That's a more confusing end-state than if we'd kept the help text conservative during the Option-3 window. The Zod schema relaxation on its own is defensible (it unblocks users who already have a broader allowlist in their agentcore.json from a previous source), but actively steering users toward X-Api-Key in the onboarding UI before deploy supports it is the part that feels risky.
Options (listing for the author's choice, in rough order of preference):
- Keep the schema relaxation +
normalizeHeaderNamepass-through, but revert the three help-text strings (AddAgentScreen.tsx:1184,GenerateWizardUI.tsx:310,AgentPrimitive.tsx:257) to the pre-PR "suffixes / Authorization" wording for now. Bump them to advertiseX-Api-Keyetc. in the follow-up PR that also bumps the@aws/agentcore-cdkpin once the mirror publishes. This keeps the CLI-side half of the fix without promising users somethingdeploycan't deliver yet. - Hold this PR entirely until the mirror change in
agentcore-l3-cdk-constructsPR [P0.5] agentcore dev --invoke fails with internal error when no creds? only on @ajesstur #209 publishes, then merge with both the help-text update and the pin bump together. This is the cleanest end-to-end story but delays the CLI-side change. - Merge as-is (status quo of this PR) — accept that the help text is ahead of what
deploysupports, and rely on the PR description / issue Relax Custom Header regex to allow new pattern #1151 to convey that to users who hit the deploy failure. This is what the PR currently does.
If you go with option 3, it would at least help to add a short inline caveat in the hint (e.g. "Note: non-Custom- headers require @aws/agentcore-cdk ≥ <ver> to deploy successfully"), though I recognize that's awkward copy for a TUI.
github-actions
Bot
removed
the
agentcore-harness-reviewing
|
Closing: fresh run with CI fix improvements |
github-actions
Bot
added
size/s
Package TarballHow to installnpm install https://github.com/aws/agentcore-cli/releases/download/pr-1163-tarball/aws-agentcore-0.13.1.tgz |
agentcore-cli-automation
left a comment
agentcore-cli-automation
left a comment
There was a problem hiding this comment.
Reviewed the PR in its current state (head c8abffa). All the serious issues I would have flagged — schema under-relaxation relative to the AWS docs, stale help/hint copy, the nonexistent ^0.1.0-alpha.29 pin, format CI failures, the assets.snapshot.test.ts drift after the pin bump, the bare-prefix validation tightening, and the end-to-end deploy-flow gap caused by the unpublished CDK mirror change — have already been raised in the existing review thread and either fixed or explicitly accepted as the documented trade-off of the Option 3 (CLI-only half-fix) scope.
No new blockers from my pass:
- Schema change correctly mirrors the linked AWS docs (RFC 7230 token characters) and the comment block accurately describes why only syntactic validation is done client-side.
normalizeHeaderName's ordering is correct — the Custom- prefix check runs before the broad AgentCore- prefix check, soX-Amzn-Bedrock-AgentCore-Runtime-Custom-Fookeeps its canonical Custom- casing rather than being truncated back to the broad prefix. TheX-pass-through branch correctly avoids silently rewritingX-Api-Key/X-Custom-Signature.- Test additions cover the three new behaviors (non-Custom AgentCore headers preserved, case canonicalization on the broad prefix, arbitrary
X-*pass-through) and don't introduce any mocking — they exercise the real functions directly. - No telemetry instrumentation is warranted here; this is a pure validation relaxation on an existing code path, not a new user-facing feature.
The one live trade-off (help text + schema advertise X-Api-Key et al. as valid, but agentcore deploy → CDK synth will still reject them until @aws/agentcore-cdk publishes the mirror change and the pin is bumped in a follow-up) is explicitly called out in both the PR description and the prior reviewer's soft-concern comment, and issue #1151 is being kept open to track the end-to-end fix. That's a reasonable way to land the CLI-side half.
LGTM.
github-actions
Bot
removed
the
agentcore-harness-reviewing
2 participants
Description
Relaxes the request header allowlist validation on the CLI side to accept the broader set of HTTP header names described in the AgentCore Runtime header allowlist documentation — e.g.
Authorization,X-Api-Key,X-Custom-Signature, plus any header in theX-Amzn-Bedrock-AgentCore-*family.Scope of this PR (Option 3 — CLI-only half-fix)
Per reviewer feedback, this PR is explicitly scoped to CLI-side validation only. The mirror change in
agentcore-l3-cdk-constructs(PR #209) has not yet been published to npm, and the currently-pinned@aws/agentcore-cdk@^0.1.0-alpha.28tarball still ships the old strict predicate.That means with this PR merged:
agentcore.jsonvalidation in the CLI accepts the broader header set.--request-header-allowlistflag and the TUI wizard advertise the new behavior.agentcore createcontinues to work (CDK pin reverted to the published^0.1.0-alpha.28).agentcore deploy→ CDK synth will still reject broader header names with the old error message until@aws/agentcore-cdkis republished with the mirror schema change and the pin insrc/assets/cdk/package.jsonis bumped.A follow-up PR will bump the
@aws/agentcore-cdkpin once the mirror change is published. Issue #1151 should stay open to track the end-to-end deploy-flow fix.Changes
src/schema/schemas/agent-env.tsHEADER_ALLOWLIST_BROAD_PREFIX(X-Amzn-Bedrock-AgentCore-) andHEADER_ALLOWLIST_PATTERN— a conservative subset of RFC 7230 token characters:/^[A-Za-z0-9!#$%&'*+\-.^_\|~]+$/`.RequestHeaderAllowlistSchema's.refine(...)predicate to validate against the new pattern; the service itself enforces the restricted-header list.HEADER_ALLOWLIST_PREFIXexported for backward compatibility.src/cli/commands/shared/header-utils.tsnormalizeHeaderNamenow passes through any header that already starts withX-(case-insensitive) unchanged, so e.g.X-Api-Keyis no longer silently rewritten toX-Amzn-Bedrock-AgentCore-Runtime-Custom-X-Api-Key. Bare suffixes (e.g.MyHeader) are still auto-prefixed with the canonical Custom- prefix for backward compatibility.HEADER_ALLOWLIST_PATTERNre-export.src/cli/commands/shared/__tests__/header-utils.test.tsnormalizeHeaderNameandvalidateHeaderAllowlistcoveringX-Api-Key,X-Custom-Signature, and non-Custom AgentCore headers (e.g.X-Amzn-Bedrock-AgentCore-Runtime-User-Id).src/cli/primitives/AgentPrimitive.tsx,src/cli/tui/screens/generate/GenerateWizardUI.tsx,src/cli/tui/screens/agent/AddAgentScreen.tsxA mirror change is being made in the
agentcore-l3-cdk-constructsrepo (PR #209). The@aws/agentcore-cdkpin insrc/assets/cdk/package.jsonwill be bumped in a follow-up PR once that publishes.Related Issue
Refs #1151 (this PR delivers the CLI-side validation portion only; the end-to-end deploy-flow fix tracks in the same issue and will be closed once the CDK constructs publish + pin bump land.)
Documentation PR
N/A — no documentation changes are required in this repo for this fix.
Type of Change
Testing
How have you tested the change?
npm run test:unitandnpm run test:integ(targeted:header-utils.test.ts— 37/37 pass)npm run typechecknpm run lint(Prettier auto-formatted)src/assets/, I rannpm run test:update-snapshotsand committed the updated snapshotsAdded unit tests covering:
normalizeHeaderNamepreserves an already-prefixed non-Custom AgentCore header unchanged (e.g.X-Amzn-Bedrock-AgentCore-Runtime-User-Id).normalizeHeaderNamecanonicalizes case for non-Custom AgentCore headers.normalizeHeaderNamepasses through otherX-headers unchanged (e.g.X-Api-Key,X-Custom-Signature).validateHeaderAllowlistaccepts non-Custom-AgentCore headers and arbitrary HTTP header names from the AWS docs.All previously-passing tests continue to pass; the existing strict-Custom canonicalization behavior is preserved.
Checklist
agentcore-l3-cdk-constructsPR [P0.5] agentcore dev --invoke fails with internal error when no creds? only on @ajesstur #209 is not yet published, and a follow-up will bump the pinBy submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the
terms of your choice.