Fix headersSchema case sensitivity for camelCase property names#2946
Fix headersSchema case sensitivity for camelCase property names#2946nick-inkeep wants to merge 5 commits intomainfrom
Conversation
…ation HTTP header names are case-insensitive (RFC 7230/9110) and are normalized to lowercase by the Fetch API. When customers defined headersSchema with camelCase properties (e.g., mcpToken), AJV validation failed because the schema expected "mcpToken" but the data had "mcptoken" — resulting in a 500 "Context validation failed" error. Extract normalizeSchemaKeysToLowercase() that lowercases all JSON Schema property names and required entries. Applied in both getCachedValidator() (for AJV compilation) and filterContextToSchemaKeys() (for post-validation filtering). Schema cache is unaffected — the cache key uses the original schema string. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 868187e The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 packages
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 |
|
TL;DR — Fixes a 500 error when customers define Key changes
Summary ｜ 4 files ｜ 5 commits ｜ base: Case-insensitive
|
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
High severity — normalizeSchemaKeysToLowercase is applied to ALL schemas, not just headers.
The fix for the header casing mismatch is correct in principle, but the normalization is placed inside getCachedValidator() — a shared utility also called from ContextFetcher.validateResponseWithJsonSchema() for response body schemas. Response bodies have case-sensitive JSON property names (e.g., userName, createdAt), so lowercasing them would cause validation mismatches. The normalization should be scoped to headers-only call sites. See inline comments for details and a minor cleanup note.
Claude Opus ｜ 𝕏
| } | ||
|
|
||
| const permissiveSchema = makeSchemaPermissive(schema); | ||
| const normalizedSchema = normalizeSchemaKeysToLowercase(schema); |
There was a problem hiding this comment.
getCachedValidator is a shared function — it's also called from ContextFetcher.ts:416 via validationHelper() to validate HTTP response bodies against definition.responseSchema. Response body property names are case-sensitive JSON keys (e.g., userName, createdAt), not HTTP headers.
By placing normalizeSchemaKeysToLowercase here, every schema compiled through this path — not just headersSchema — gets its property names lowercased. This would silently break response schema validation for any schema with camelCase property names.
Fix: Move the normalization out of getCachedValidator and into the two headers-specific call sites:
- In
validateHttpRequestHeaders(line 239), normalize before callingvalidationHelper - In
filterContextToSchemaKeys(line 202), normalize before callingfilterByJsonSchema— this is already done correctly
This also eliminates the double-normalization: currently the schema is normalized once in getCachedValidator (for AJV) and again in filterContextToSchemaKeys (for filtering).
| const normalizedSchema = normalizeSchemaKeysToLowercase(schema); | ||
| const permissiveSchema = makeSchemaPermissive(normalizedSchema); |
There was a problem hiding this comment.
Remove normalization from getCachedValidator and keep it headers-only. The normalizeSchemaKeysToLowercase call should happen in validateHttpRequestHeaders before the schema reaches this function.
| const normalizedSchema = normalizeSchemaKeysToLowercase(schema); | |
| const permissiveSchema = makeSchemaPermissive(normalizedSchema); | |
| const permissiveSchema = makeSchemaPermissive(schema); |
| } | ||
|
|
||
| const filteredHeaders = filterByJsonSchema(validatedContext, headersSchema); | ||
| const normalizedSchema = normalizeSchemaKeysToLowercase(headersSchema); |
There was a problem hiding this comment.
This normalization is correct for the filtering path. After moving normalization out of getCachedValidator, you could normalize once at the top of validateHttpRequestHeaders and pass the normalized schema to both validationHelper and filterContextToSchemaKeys, avoiding the redundant second normalization here.
| url: '#get-evaluators-by-ids' | ||
| - depth: 2 | ||
| title: Batch get agent scopes for evaluators | ||
| url: '#batch-get-agent-scopes-for-evaluators' |
There was a problem hiding this comment.
This change (adding "Batch get agent scopes for evaluators" to the TOC and operations list) is unrelated to the header schema fix. The file header says This file was generated by Fumadocs. Do not edit this file directly. — this was likely regenerated as a side-effect of a build/rebase. Consider removing it from this PR to keep the diff focused.
| @@ -0,0 +1,114 @@ | |||
| # Normalize headersSchema Property Names to Lowercase | |||
There was a problem hiding this comment.
Minor: the spec proposes extending makeSchemaPermissive() to include the lowercasing logic (lines 41–71), but the implementation chose a separate normalizeSchemaKeysToLowercase() function instead. The separate function is a better design (single responsibility), but the spec should be updated to match what was actually implemented — or removed from the PR if it was only a planning artifact.
Move normalizeSchemaKeysToLowercase() out of getCachedValidator() (shared by all schemas) and into validateHttpRequestHeaders() (header-specific). getCachedValidator is also used by ContextFetcher.validateResponseWithJsonSchema for API response validation — lowercasing those property names would break case-sensitive API responses (e.g., userId → userid). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Review Summary
(2) Total Issues | Risk: Low
🟡 Minor (1) 🟡
Inline Comments:
- 🟡 Minor:
contextValidation.headers.test.ts:181Missing test for nested object schemas with camelCase (SPEC.md Test Case #5)
💭 Consider (1) 💭
Inline Comments:
- 💭 Consider:
validation.ts:78Schema without explicittype: 'object'would skip normalization
🧹 While You're Here (1) 🧹
🧹 1) evaluations.mdx Unrelated auto-generated docs change
Issue: This file change (agents-docs/content/api-reference/(openapi)/evaluations.mdx) adds a new "Batch get agent scopes for evaluators" entry but is unrelated to the headersSchema case sensitivity fix.
Why: The file header says "This file was generated by Fumadocs. Do not edit this file directly." This appears to be incidental regeneration bundled with the PR. Consider separating into its own commit or PR for cleaner history.
Fix: Either remove from this PR (revert and let the next docs regeneration pick it up) or add a note in the PR description that this is an unrelated but legitimate auto-generated change.
Refs: evaluations.mdx diff
Discarded (2)
| Location | Issue | Reason Discarded |
|---|---|---|
validation.ts:81 |
Key collision when two properties differ only by case | HTTP headers are case-insensitive per RFC 7230/9110 — customers defining conflicting case variants (e.g., McpToken and mcpToken) would be a user error. The "last wins" behavior is acceptable for this edge case. |
contextValidation.headers.test.ts:182 |
Missing test for anyOf/oneOf composition schemas | Composition schemas are uncommon for HTTP headers. The code handles them correctly and the existing tests provide sufficient coverage for the primary use case. |
Reviewers (4)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-tests |
4 | 0 | 0 | 0 | 1 | 0 | 2 |
pr-review-standards |
2 | 0 | 0 | 0 | 1 | 0 | 1 |
pr-review-errors |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
pr-review-precision |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
| Total | 6 | 0 | 0 | 0 | 2 | 0 | 3 |
Note: Error handling and precision reviewers found no issues — the implementation is clean and well-targeted.
💡 APPROVE WITH SUGGESTIONS
Summary: This is a well-executed bug fix that correctly addresses the root cause of the case sensitivity mismatch between headersSchema property names and lowercased HTTP headers. The implementation is minimal, precisely targeted, and follows the existing patterns in the codebase. The new normalizeSchemaKeysToLowercase function is clean and handles JSON Schema composition keywords correctly. The test coverage is solid for the primary use cases. Minor suggestions above are optional improvements rather than blocking concerns. Nice work! 🎉
|
|
||
| const normalized = { ...schema }; | ||
|
|
||
| if (normalized.type === 'object') { |
There was a problem hiding this comment.
💭 Consider: Schema without explicit type: 'object'
Issue: This check requires type === 'object' explicitly, but JSON Schema allows properties without an explicit type declaration.
Why: A schema like { properties: { McpToken: { type: 'string' } } } (without type: 'object') would skip property normalization. However, the practical risk is low since all existing schemas in the codebase use type: 'object' explicitly, and this matches the pattern in makeSchemaPermissive() (line 118).
Fix: If you want to be maximally defensive, consider:
if (normalized.type === 'object' || normalized.properties)Refs:
- makeSchemaPermissive uses same pattern — consistency is maintained
| authtoken: 'bearer-xyz', | ||
| }); | ||
| expect((result.validatedContext as any)?.['extra-header']).toBeUndefined(); | ||
| }); |
There was a problem hiding this comment.
🟡 Minor: Missing test for nested object schemas with camelCase
Issue: The normalizeSchemaKeysToLowercase function recursively processes nested object properties (line 82 in validation.ts), but no test verifies that deeply nested camelCase properties are correctly lowercased.
Why: This is explicitly called out in SPEC.md as Test Case #5 ("Nested object schemas: Schema with nested object properties using camelCase → properties lowercased at all levels"). While header values are typically strings, the recursive logic exists and should be tested for confidence.
Fix: Consider adding a test like:
it('should normalize nested object schema properties', async () => {
const headersSchema = {
type: 'object',
properties: {
authConfig: {
type: 'object',
properties: {
accessToken: { type: 'string' },
},
},
},
};
// Verify the schema compiles and validates without error
const httpRequest: ParsedHttpRequest = {
headers: {
authconfig: 'some-value',
},
};
const result = await validateHttpRequestHeaders(headersSchema, httpRequest);
expect(result.valid).toBe(true);
});Refs:
There was a problem hiding this comment.
PR Review Summary
(0) New Issues | Risk: Low | Delta Review
This review covers the 1 commit since the last automated review (56d3f74de → 91ad5b67b).
✅ Delta Assessment
The latest commit (91ad5b67b Fix: scope schema key normalization to header schemas only) correctly addresses the HIGH severity issue raised in the prior pullfrog review.
What changed:
- const normalizedSchema = normalizeSchemaKeysToLowercase(schema);
- const permissiveSchema = makeSchemaPermissive(normalizedSchema);
+ const permissiveSchema = makeSchemaPermissive(schema);- const validate = validationHelper(headersSchema);
+ const normalizedHeadersSchema = normalizeSchemaKeysToLowercase(headersSchema);
+ const validate = validationHelper(normalizedHeadersSchema);Why this is correct:
getCachedValidator()is a shared utility used by bothvalidateHttpRequestHeaders()(for HTTP headers) andContextFetcher.validateResponseWithJsonSchema()(for response bodies)- HTTP headers are case-insensitive per RFC 7230/9110 and should be normalized
- Response body JSON properties (e.g.,
userName,createdAt) are case-sensitive and should NOT be normalized - The fix correctly scopes normalization to the headers-only call site
🕐 Pending Recommendations (3)
Items from prior reviews still applicable:
- 🟡
contextValidation.headers.test.tsMissing test for nested object schemas with camelCase (SPEC.md Test Case #5) - 💭
validation.ts:78Schema without explicittype: 'object'would skip normalization - 🧹
evaluations.mdxUnrelated auto-generated docs change bundled in PR
✅ APPROVE
Summary: The delta successfully addresses the critical scoping issue from the prior review. The normalization is now correctly limited to header schemas only, ensuring response body validation remains case-sensitive. The implementation is minimal and precisely targeted. The prior suggestions (nested object test, type guard edge case, unrelated docs change) remain as optional improvements but are not blocking. Ship it! 🚀
Reviewers (2)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-standards |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
pr-review-precision |
1 | 0 | 0 | 0 | 0 | 1 | 0 |
| Total | 1 | 0 | 0 | 0 | 0 | 1 | 0 |
Note: The precision reviewer identified redundant normalization (schema normalized twice per request), but this was already noted in the prior pullfrog review and is a minor optimization opportunity, not a bug.
|
Addressing review feedback: Pullfrog (HIGH — shared getCachedValidator): Already fixed in commit Claude — nested object test (Minor): Valid suggestion per spec test case #5. In practice, HTTP headers are flat key-value strings (not nested objects), so the recursive path is defensive code. The normalization function handles it correctly — adding a test for completeness would be a nice-to-have but not blocking. Claude — type guard edge case (Consider): Matches existing Claude + Pullfrog — evaluations.mdx (While You're Here): This is auto-generated by Fumadocs during build. Will revert from this PR to keep the diff focused. |
Remove auto-generated Fumadocs change that was incidentally bundled in this PR. Keeps the diff focused on the headersSchema fix. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
PR Review Summary
(0) New Issues | Risk: Low | Delta Review
This review covers the 1 commit since the last automated review (91ad5b67b → 868187e3e).
✅ Delta Assessment
The latest commit (868187e3e chore: revert unrelated evaluations.mdx auto-generated change) addresses the While You're Here item from prior reviews.
What changed:
- Reverted the incidentally bundled
evaluations.mdxauto-generated docs change - Keeps the PR diff focused on the
headersSchemacase sensitivity fix
This completes the feedback loop — all actionable items from prior reviews have been addressed.
📋 Prior Feedback Summary
| Issue | Status |
|---|---|
🔴 Pullfrog HIGH — normalization in shared getCachedValidator() |
✅ Fixed in 91ad5b67b |
| 🧹 evaluations.mdx — unrelated docs change | ✅ Reverted in 868187e3e (this delta) |
| 🟡 Nested object test — SPEC.md Test Case #5 | ℹ️ Author acknowledged; HTTP headers are flat strings, recursive path is defensive code |
💭 Type guard edge case — type === 'object' check |
ℹ️ Author noted consistency with makeSchemaPermissive() pattern |
🔍 Final Code Assessment
The implementation is correct and well-scoped:
- Root cause fixed:
normalizeSchemaKeysToLowercase()correctly lowercases JSON Schema property names andrequiredentries before AJV validation - Scoping correct: Normalization is applied only to
headersSchemainvalidateHttpRequestHeaders(), not in the sharedgetCachedValidator()which is also used for case-sensitive response body schemas - Test coverage solid: 4 new test cases cover camelCase properties, required entries, mixed case, and filtering behavior
- No regression risk: Existing lowercase schemas continue to work identically
✅ APPROVE
Summary: This PR correctly fixes the case sensitivity mismatch between headersSchema property names and lowercased HTTP headers. The implementation is minimal, precisely targeted, and follows HTTP semantics (RFC 7230/9110). All prior review feedback has been addressed. Ship it! 🚀
Reviewers (0)
No new reviewers dispatched for this delta — the only change was a revert of the docs file as previously requested.
1 participant
Summary
headersSchemaproperty names andrequiredentries to lowercase before AJV validation and key filteringmcpToken) that don't match lowercased HTTP header keys (mcptoken)Changes
validation.tsnormalizeSchemaKeysToLowercase()— recursively lowercases JSON Schema property names andrequiredentriesvalidateHttpRequestHeaders()andfilterContextToSchemaKeys()only (header-specific call sites)getCachedValidator()— that function is also used byContextFetcher.validateResponseWithJsonSchema()for API response schemas where property names are case-sensitivecontextValidation.headers.test.tsOther
agents-api(patch)evaluations.mdxchangeRoot cause
contextValidationMiddlewarecorrectly lowercases incoming header keys (key.toLowerCase()), but theheadersSchemaproperty names were NOT lowercased before validation. AJV's property matching is case-sensitive, so{ properties: { "mcpToken" } }didn't match data key"mcptoken". Additionally,filterContextToSchemaKeysiterates schema properties and checkskey in data— also case-sensitive.Blast radius
normalizeSchemaKeysToLowercaseis scoped to header validation onlygetCachedValidator()(shared by ContextFetcher for API response validation) is NOT affected — response schemas with camelCase properties (e.g.,userId) continue to work correctlyTest plan
🤖 Generated with Claude Code