Skip to content

Fix schema redaction alias collisions - #402

Merged
mohanagy merged 2 commits into
developmentfrom
fix/399-schema-key-collisions
Aug 12, 2026
Merged

Fix schema redaction alias collisions#402
mohanagy merged 2 commits into
developmentfrom
fix/399-schema-key-collisions

Conversation

@mohanagy

@mohanagy mohanagy commented Aug 12, 2026

Copy link
Copy Markdown
Owner

Summary

  • preserve every schema entry when distinct sensitive keys redact to the same marker
  • assign deterministic collision-free aliases once per schema document
  • reuse those aliases across properties, definitions, $ref, required/dependency references, defaults, and examples
  • add a two-key regression fixture and release-note coverage

Closes the valid release-blocking review finding on #401. Supports #397 and #399.

Security impact

Secret-bearing key material remains absent from client-visible catalogs. The change prevents silent schema-entry loss while keeping alias references coherent and collision-free.

Validation

  • npm run check:test-fixture
  • npm run typecheck
  • npm run lint
  • npx vitest run tests/mcp-wrapper.test.ts (101 passed)
  • npm test (158 files; 1,948 passed, 34 skipped)
  • npm run build
  • npm run test:inspector (MCP Inspector 2.1.0)
  • changelog/release contracts (8 passed)

Publication boundary

This PR targets development only. It does not publish. v1.1.1 remains gated on exact developmentmain promotion, merged-main CI, and the protected GitHub Release/OIDC workflow.

Summary by CodeRabbit

  • Bug Fixes

    • Sensitive schema keys now receive stable, collision-free aliases.
    • Aliases are applied consistently across schema definitions, references, nested structures, dependencies, and examples.
    • Client-visible schema values of true are converted to {} for improved Claude Desktop compatibility.
    • Existing redaction and boolean-schema behavior remains unchanged.
  • Tests

    • Expanded schema compatibility coverage for references, definitions, dependencies, required fields, defaults, and examples.

@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 9 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 9f757aca-c4ac-42dd-ab93-3332e7a58c2d

📥 Commits

Reviewing files that changed from the base of the PR and between a6cab0b and cab6433.

📒 Files selected for processing (1)
  • tests/mcp-wrapper.test.ts
📝 Walkthrough

Walkthrough

The change adds shared, collision-free aliases for sensitive schema keys and rewrites related references consistently. It also converts client-visible schema true values to {} for Claude Desktop compatibility. Tests now cover the complete transformed Vercel schema.

Changes

Schema redaction compatibility

Layer / File(s) Summary
Shared schema redaction context
src/mcp/server/miftah-server.ts, CHANGELOG.md
Schema redaction assigns unique aliases and applies them to keys, references, nested values, dependencies, arrays, and literals. Boolean schema handling remains unchanged. The changelog records the compatibility behavior.
Vercel schema compatibility coverage
tests/fixtures/fake-upstream-runtime.mjs, tests/mcp-wrapper.test.ts
The fixture adds referenced definitions, bidirectional dependencies, required fields, and expanded examples. The test asserts the complete transformed schema.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Poem

I’m a rabbit with aliases bright,
Redacting keys both day and night.
References hop in matching rows,
Nested schemas bloom like does.
True becomes {} with cheer—
Claude Desktop works clear!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the primary change: fixing collisions in schema redaction aliases.
Description check ✅ Passed The description covers the focused change, security impact, and validation results, with useful publication-boundary context.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/399-schema-key-collisions

Comment @coderabbitai help to get the list of available commands.

@mohanagy

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@mohanagy

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai[bot]
coderabbitai Bot previously requested changes Aug 12, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/mcp-wrapper.test.ts`:
- Around line 3913-3917: Update the fixture assertions in the test around the
default and examples entries to verify preservation of the bearer-redacted
default entry, asserting its collision-free alias "[REDACTED_3]" with the
expected value "bearer-key" or adding an exact nested default assertion. Ensure
the matcher cannot pass when this entry is omitted.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 8298f8c8-d158-4fd9-86e7-8ba55be9ae19

📥 Commits

Reviewing files that changed from the base of the PR and between e43f8ab and a6cab0b.

📒 Files selected for processing (5)
  • CHANGELOG.md
  • src/mcp/server/miftah-server.ts
  • tests/fixtures/fake-upstream-bundled.mjs
  • tests/fixtures/fake-upstream-runtime.mjs
  • tests/mcp-wrapper.test.ts

Comment thread tests/mcp-wrapper.test.ts
@mohanagy
mohanagy dismissed coderabbitai[bot]’s stale review August 12, 2026 08:12

Finding fixed in cab6433. CodeRabbit confirmed it addressed on the current head and automatically resolved the only review thread.

@mohanagy
mohanagy merged commit 1b5e306 into development Aug 12, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant