fix(schema): relax requestHeaderAllowlist validation to match runtime contract#1155
fix(schema): relax requestHeaderAllowlist validation to match runtime contract#1155aidandaly24 wants to merge 1 commit into
Conversation
github-actions
Bot
added
size/m
Coverage Report
|
agentcore-cli-automation
left a comment
agentcore-cli-automation
left a comment
There was a problem hiding this comment.
Thanks for putting together the docs/tests for this — the schema relaxation clearly matches the AgentCore Runtime contract now.
I flagged a few things I think should be resolved before merging, all around cross-cutting consequences of removing the auto-prefix behavior from normalizeHeaderName. Nothing wrong with the schema logic itself; the concerns are about (a) the L3 CDK construct still rejecting the new headers at synth/deploy time, (b) an unannounced behavior change for agentcore invoke -H / agentcore dev -H, and (c) missing schema-level tests. Details inline.
| } | ||
| seen.add(k); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Deploy-time schema drift with @aws/agentcore-cdk.
The vended CDK project (src/assets/cdk/package.json) depends on @aws/agentcore-cdk at ^0.1.0-alpha.19, which re-validates the spec at synth time using its own copy of RequestHeaderAllowlistSchema. Until the companion change in aws/agentcore-l3-cdk-constructs is merged and a new version is published to npm, any new-style allowlist entry (X-Api-Key, X-Custom-Signature, Some_Header_With_Underscores, …) that passes CLI validation will fail at cdk synth / agentcore deploy with the old strict-prefix error — a confusing regression for end users.
Possible resolutions (pick one):
- Hold this PR until the CDK companion is merged and a version bump is published, then bump
@aws/agentcore-cdkinsrc/assets/cdk/package.jsonto a version guaranteed to include the relaxation. - Pin
@aws/agentcore-cdkto an exact pre-release that contains the fix (avoids a cached caret range pulling a stale version). - If the duplicated
src/schema/schemas/agent-env.tshere is treated as the authoritative runtime validator pre-CDK, add an upfront check that the resolved@aws/agentcore-cdkversion supports the relaxed schema so users get a clear error rather than a deep synth-time Zod stack.
At minimum the PR description should call out the published-version dependency, and CI in this repo should be exercising the e2e path against a CDK tarball that already has the fix (e.g. via npm run bundle) so this doesn't silently regress after merge.
| return `${HEADER_ALLOWLIST_PREFIX}${input.slice(HEADER_ALLOWLIST_PREFIX.length)}`; | ||
| } | ||
| return `${HEADER_ALLOWLIST_PREFIX}${input}`; | ||
| return input; |
There was a problem hiding this comment.
Breaking behavior change for agentcore invoke -H and agentcore dev -H.
Because parseHeaderFlag (→ normalizeHeaderName) is on the hot path for the -H / --header flags in both src/cli/commands/invoke/command.tsx and src/cli/commands/dev/command.tsx, and the resulting headers are forwarded as literal HTTP headers to the runtime (see invoke/action.ts lines 161/245/325/369/426/460), this change alters what gets sent on the wire. Previously:
agentcore invoke -H "MyHeader: val"
sent X-Amzn-Bedrock-AgentCore-Runtime-Custom-MyHeader: val. After this PR it sends MyHeader: val verbatim. Users with an existing deployed runtime whose allowlist still contains X-Amzn-Bedrock-AgentCore-Runtime-Custom-MyHeader will silently stop receiving the header at the agent — the invoke will appear to succeed but the agent won't see the value. This is a behavior break that is distinct from the schema relaxation and isn't called out in the PR description or CHANGELOG.md.
Options to address:
- Preserve auto-prefix only in
parseHeaderFlag(the invoke/dev CLI path) and remove it only from the allowlist-normalization path. The two paths don't actually need to share normalization — one configures the runtime, the other talks to it. - Keep this change but add an explicit "Breaking changes" section to the PR body +
CHANGELOG.md, and ideally haveparseHeaderFlagemit a one-time warning when a bare name (notAuthorization, notX-Amzn-Bedrock-…-Custom-) is passed via-H, suggesting the user either add the full prefix or the literal name to their allowlist. - If (1) is rejected because we want the CLI to always send what the user typed, at least document the migration explicitly so users don't debug silently-dropped headers.
| if (reason) { | ||
| ctx.addIssue({ code: z.ZodIssueCode.custom, message: reason }); | ||
| } | ||
| }) |
There was a problem hiding this comment.
Missing unit tests for RequestHeaderAllowlistSchema itself.
All new coverage is in header-utils.test.ts, which exercises the wrapper utilities. The Zod schema's inner superRefine (here) and the array-level duplicate detection on lines 328–341 are the last line of defense when a user hand-edits agentcore/agentcore.json or imports a config that wasn't produced by the TUI/CLI — but src/schema/schemas/__tests__/agent-env.test.ts has nothing for RequestHeaderAllowlistSchema today.
Please add safeParse tests that at minimum cover:
- accepts
['Authorization'],['X-Amzn-Bedrock-AgentCore-Runtime-Custom-Foo'],['X-Api-Key'],['Some_Header_With_Underscores']; - rejects
['Cookie'],['cookie'],['X-Amz-Date'],['X-Amzn-Foo'],['Bad Header'],['Bad@Header'],['']; - rejects duplicates at the array level — e.g.
['MyHeader', 'myheader']— and verify the issue'spathpoints at the duplicate index (the schema emitspath: [i], and that contract is worth pinning); - rejects arrays of length 21.
The util-layer tests are valuable but don't prove the schema itself behaves correctly when config comes from a path other than the TUI/CLI.
| error: `Duplicate header (case-insensitive): "${raw}".`, | ||
| }; | ||
| } | ||
| seen.add(key); |
There was a problem hiding this comment.
Minor: validateHeaderAllowlist now computes headers = parseAndNormalizeHeaders(value) on line 77 (which already dedups case-insensitively), then runs a second manual dedup loop over rawNames on lines 79–88 to produce the duplicate-rejection error. Only headers.length from the first call is used after that. Functionally fine today, but the two passes will silently disagree if parseAndNormalizeHeaders's dedup semantics ever change. Worth collapsing to a single enumeration of rawNames that produces both the count and the duplicate diagnostic — especially since there are no direct tests for the schema's dedup yet (see my other comment).
github-actions
Bot
removed
the
agentcore-harness-reviewing
|
Closing: duplicate from batch test run |
2 participants
Description
Relaxes the client-side validation for
requestHeaderAllowlistto match theexpanded contract described in the AgentCore Runtime docs:
https://docs.aws.amazon.com/bedrock-agentcore/latest/devguide/runtime-header-allowlist.html
Previously, the CLI/CDK Zod schema rejected any header that wasn't either
Authorizationor prefixed withX-Amzn-Bedrock-AgentCore-Runtime-Custom-.The runtime now accepts any non-restricted HTTP header name (alphanumerics,
hyphens, underscores), so the strict prefix check produced false negatives for
valid headers like
X-Custom-Signature,X-Api-Key, etc.What changed
src/schema/schemas/agent-env.ts(mirrored in the CDK repo).refine()rule with asuperRefine()that rejects:/^[A-Za-z0-9_-]+$/(now allows underscores),Cookie,Host,Accept,Content-Type, etc.),x-amz-*(reserved for SigV4),x-amzn-*other than the AgentCore custom prefix.Authorizationand any header starting with the AgentCore custom prefixremain explicitly allowed (back-compat).
superRefine()that rejects case-insensitive duplicates.HEADER_NAME_REGEX,RESTRICTED_HEADER_NAMES, and agetHeaderRejectionReason(name)helper for shared reuse.src/cli/commands/shared/header-utils.tsnormalizeHeaderNameno longer auto-prepends the AgentCore custom prefix toarbitrary names. It only canonicalizes
Authorizationcasing and the legacyAgentCore custom prefix casing.
validateHeaderAllowlistnow delegates togetHeaderRejectionReasonso theCLI/TUI surface the same rules as the schema (restricted-list, reserved
prefixes, duplicate detection).
Tests (
src/cli/commands/shared/__tests__/header-utils.test.ts)MyHeader→MyHeader,not auto-prefixed).
X-Custom-Signature,X-Api-Key, underscore-containingnames, restricted headers (
Cookie,Host,Accept,Content-Type,User-Agent,Connection),x-amz-*,x-amzn-*non-allowlist,case-insensitive duplicates. 38 tests pass (was 27).
Help text & docs
src/cli/primitives/AgentPrimitive.tsx— updated--request-header-allowlistdescription.
src/cli/tui/screens/agent/AddAgentScreen.tsx,src/cli/tui/screens/generate/GenerateWizardUI.tsx— updated dim-textguidance under the allowlist prompt.
docs/configuration.md— refreshedrequestHeaderAllowlistrow with alink to the AWS docs page.
Companion CDK change
The matching schema change for the L3 constructs lives in
aws/agentcore-l3-cdk-constructson branchfix/1151-93d27c63(commit
a62396b) and must merge alongside this PR to keep the duplicatedsrc/schema/schemas/agent-env.tsin sync.Related Issue
Closes #1151
Documentation PR
N/A — the AWS Bedrock AgentCore developer guide already documents the
relaxed contract this PR aligns with.
Type of Change
Testing
How have you tested the change?
npm run test:unitandnpm run test:integheader-utils.test.ts(38/38),agent-env.test.ts(85/85),schema-mapper.test.ts,import-runtime-handler.test.ts, andagentcore-control.test.ts(101/101 across the latter three) all pass. Full suite to be exercised
by CI.
npm run typecheck— clean in both repos.npm run lint— pre-commit lint-staged hook ran clean on everymodified file.
src/assets/, I rannpm run test:update-snapshotsandcommitted the updated snapshots — N/A, no snapshot fixtures touched.
New test coverage
X-Custom-Signature,X-Api-Key,Some_Header_With_Underscores,case-insensitive
Authorization, mixed allow-listed entries.User-Agent/Connection), case-insensitive restricted check,
X-Amz-Date,X-Amzn-Foo, duplicate detection (MyHeader, myheader).Checklist
feature works
feature, or no new docs are needed
branch ready for review/merge alongside this PR.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the
terms of your choice.