feat(profiles): add stateless profile context handles - #380
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR adds encrypted, authenticated, revocable profile-context handles. Modern MCP requests resolve and sanitize these handles across tools, resources, prompts, and profile operations. Audit correlation, runtime options, public exports, documentation, and compatibility tests are updated. ChangesModern stateless profile-context boundary
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Suggested labels: Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
coderabbitai
Bot
added
area:mcp
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/mcp/server/miftah-server.ts (2)
1007-1041: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftModern profile-context rejections bypass the audit journal in every MCP handler. Handle resolution and request authentication run before
runAuditedopens an audit scope. A failed authentication, or a tampered, expired, or revoked handle, therefore terminates the request with no audit event. The modern mode requires a configured audit journal, and the constructor enforces that at lines 370 to 378, so a silent rejection contradicts the stated guarantee and hides handle-probing activity from operators.
src/mcp/server/miftah-server.ts#L1007-L1041: open the audit scope beforeprepareCall, and finish it with the normalized error code whenprepareCallthrows, instead of returningtextResultat line 1013.src/mcp/server/miftah-server.ts#L1063-L1103: move themodernRequestProfileStatecall insiderunAudited, or open a scope beforehand, for the resource-template and subscribe handlers, and apply the same change at lines 1133, 1163, 1200, 1230, and 1267.Use the configured default profile as
sourceProfilewhen no handle resolves, because the caller's profile is unknown at that point.🤖 Prompt for 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. In `@src/mcp/server/miftah-server.ts` around lines 1007 - 1041, The MCP handlers must audit modern profile-context failures before request preparation or authentication can reject them. In src/mcp/server/miftah-server.ts:1007-1041, open runAudited before prepareCall, use the configured default profile as sourceProfile when resolution fails, and complete the audit with the normalized error code instead of returning directly; in src/mcp/server/miftah-server.ts:1063-1103 and the handlers at lines 1133, 1163, 1200, 1230, and 1267, move modernRequestProfileState inside runAudited or open an equivalent scope beforehand, applying the same normalized rejection auditing.
977-1005: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRequire strict discovery for modern profile contexts.
tools/listadvertises the default or locked profile, whiletools/callresolves the handle's profile. Since"permissive"is the default, a non-default handle can receiveTOOL_NOT_FOUNDfor an advertised tool. Force"strict"mode or require identical client-visible tool names and schemas before enabling modern profile contexts.🤖 Prompt for 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. In `@src/mcp/server/miftah-server.ts` around lines 977 - 1005, Update registerHandlers and the modern profile-context discovery flow so tools/list uses strict discovery rather than advertising permissive/default-profile tools; alternatively, gate modern profile contexts until client-visible tool names and schemas are identical across profiles. Ensure tools/call cannot resolve a handle profile that differs from the profile used to advertise the tool.
🤖 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 `@src/mcp/server/miftah-server.ts`:
- Around line 3327-3334: Update the sanitized argument construction in the loop
building args to avoid prototype-mutating assignment for payload keys such as
__proto__. Initialize args with a null prototype or copy each key via
Object.defineProperty, while preserving the existing filtering of
PROFILE_CONTEXT_ARGUMENT and downstream frozen-object behavior.
- Around line 1867-1887: Bind modern profile transition approvals to the
authenticated request context. Update the call site of
requireModernProfileTransitionConfirmation to pass modern.authenticated, extend
that method’s context handling as needed, and include
modern.authenticated.auditCorrelation in the requireApproval binding alongside
the existing profile fields so concurrent requests cannot reuse each other’s
approvals.
- Around line 946-962: Update normalizeProfileContextError so only unrecognized
errors that are instances of ProfileContextHandleError map to
PROFILE_CONTEXT_INVALID; classify all other unknown errors, including plain
infrastructure Error values, as PROFILE_CONTEXT_UNAVAILABLE. Preserve the
existing mappings for known context error codes and MiftahError instances.
In `@src/profiles/profile-context-handle.ts`:
- Around line 368-377: Update the interface documentation for keyringProvider
near lines 68–69 to state that it is invoked once per mint, resolve, and revoke
operation and twice during replace, so hosts must serve a short-TTL cached
keyring snapshot and enforce their own timeout. Do not change the keyring()
implementation.
- Around line 381-392: Document in the key-manager operating procedure that
ProfileContextHandle’s highestActiveEpoch and highestActiveKey protections are
limited to a single process instance and reset on restart. State that the key
manager must reject epoch rollbacks and same-epoch key replacement at the
source, and define monitoring for epoch regressions.
- Around line 441-448: Update revoke to call the existing prune method before
checking maximumEntries, using Date.now() as the pruning timestamp. Keep the
subsequent capacity check and expirations.set behavior unchanged so expired
entries are reclaimed before deciding that capacity is unavailable.
In `@tests/profile-context-handle.test.ts`:
- Around line 243-273: Add a test covering revocation-store failure within
replace after commitAudit succeeds. Configure shared keyring and a
revocations.revoke failure, assert replace rejects with
PROFILE_CONTEXT_UNAVAILABLE, verify the audit callback ran, and confirm the
prior handle remains usable without exposing a replacement.
- Line 499: Replace the probabilistic minted.handle substring assertion in the
relevant profile-handle test with a deterministic property check: decode the
ciphertext segment and verify the plaintext profile name is absent from the raw
bytes, or reuse the existing randomBytes seam to mint a fixed handle. Preserve
the test’s intent of verifying that the profile name is not exposed in
plaintext.
- Around line 115-119: Update the assertion loops over invalidInputs and the
corresponding loops near the other referenced locations to include the current
input’s index or a descriptive label in each assertion, so failures identify
which case failed while preserving the existing validation expectations.
- Around line 406-414: Add malformed test cases to the `malformed` array using
valid base64url strings that decode to incorrect byte lengths for the
initialization-vector and authentication-tag parts, rather than `"!"`. Ensure
these cases exercise the `decodePart` exact-length validation for both envelope
components while preserving the existing malformed inputs.
---
Outside diff comments:
In `@src/mcp/server/miftah-server.ts`:
- Around line 1007-1041: The MCP handlers must audit modern profile-context
failures before request preparation or authentication can reject them. In
src/mcp/server/miftah-server.ts:1007-1041, open runAudited before prepareCall,
use the configured default profile as sourceProfile when resolution fails, and
complete the audit with the normalized error code instead of returning directly;
in src/mcp/server/miftah-server.ts:1063-1103 and the handlers at lines 1133,
1163, 1200, 1230, and 1267, move modernRequestProfileState inside runAudited or
open an equivalent scope beforehand, applying the same normalized rejection
auditing.
- Around line 977-1005: Update registerHandlers and the modern profile-context
discovery flow so tools/list uses strict discovery rather than advertising
permissive/default-profile tools; alternatively, gate modern profile contexts
until client-visible tool names and schemas are identical across profiles.
Ensure tools/call cannot resolve a handle profile that differs from the profile
used to advertise the tool.
🪄 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: 3acf2ca3-e096-476d-8341-8f0eea13160d
📒 Files selected for processing (21)
CHANGELOG.mddocs/library-api.mddocs/plans/2026-08-11-stateless-profile-context-decision.mdpackage.jsonsrc/audit/audit-trail.tssrc/audit/audit-types.tssrc/cli/exit-codes.tssrc/index.tssrc/mcp/server/miftah-server.tssrc/mcp/server/operation-pipeline.tssrc/profiles/profile-context-handle.tssrc/profiles/profile-manager.tssrc/runtime/create-miftah-runtime.tssrc/utils/errors.tstests/cli-exit-codes.test.tstests/package-contract.test.tstests/profile-context-handle-docs-contract.test.tstests/profile-context-handle.test.tstests/public-api.test.tstests/stateless-profile-context-runtime.test.tsvitest.config.ts
Owner
Author
|
Addressed the stateless-context review in 9446f6c.
Validation: npm test (153 files; 1,900 passed, 34 skipped), npm run test:coverage (95.97% lines / 92.15% branches overall; profile-context handle 98.53% lines / 94.19% branches / 100% functions), typecheck, lint, build, check:pack, and test:package (31/31) all pass locally. |
There was a problem hiding this comment.
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/stateless-profile-context-runtime.test.ts`:
- Around line 286-292: Add an explicit assertion in the prototypeArguments test
that the relevant object prototype remains unpolluted after calling the echo
tool, alongside the existing echoed-text assertion. Target the behavior
protected by extractProfileContext and verify the injected polluted property is
absent rather than relying only on the response content.
🪄 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: bb467dae-513e-45a8-b6a1-c3254dceb099
📒 Files selected for processing (8)
CHANGELOG.mddocs/library-api.mddocs/plans/2026-08-11-stateless-profile-context-decision.mdsrc/audit/audit-trail.tssrc/mcp/server/miftah-server.tssrc/profiles/profile-context-handle.tstests/profile-context-handle.test.tstests/stateless-profile-context-runtime.test.ts
Owner
Author
|
Final review follow-up is in |
mohanagy
dismissed stale reviews from coderabbitai[bot] and coderabbitai[bot]
August 11, 2026 12:12
All actionable findings were addressed on later commits; exact-head CodeRabbit and CI checks are green, and all review threads are resolved.
8 tasks
This was referenced Aug 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #377
Outcome
Validation
Summary by CodeRabbit
New Features
Documentation
Tests