fix(mcp): Fix sandbox read-only mode not being applied to MCP sessions#57306
Open
harsh21234i wants to merge 2 commits intoPostHog:masterfrom
Open
fix(mcp): Fix sandbox read-only mode not being applied to MCP sessions#57306harsh21234i wants to merge 2 commits intoPostHog:masterfrom
harsh21234i wants to merge 2 commits intoPostHog:masterfrom
Conversation
Contributor
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
services/mcp/tests/unit/readonly-header.test.ts:5-27
**Prefer parameterised tests**
All three cases test the same function with different inputs and the same expected output — this is exactly the pattern `it.each` is designed for. The Python side of this very PR uses `@parameterized.expand` correctly; the TypeScript side should follow the same convention.
```suggestion
it.each([
['canonical x-posthog-readonly header', { 'x-posthog-readonly': 'true' }, ''],
['legacy x-posthog-read-only header', { 'x-posthog-read-only': '1' }, ''],
['readonly query parameter', {}, '?readonly=true'],
])('reads read-only from %s', (_, headers, qs) => {
const request = new Request(`https://mcp.posthog.com/mcp${qs}`, { headers })
expect(getReadOnlyFromRequest(request, new URL(request.url))).toBe(true)
})
```
Reviews (1): Last reviewed commit: "fix(mcp): align sandbox readonly header" | Re-trigger Greptile |
Contributor
Author
|
hey @haacked have you checked this? |
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
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.
Summary
This fixes a header mismatch between the task sandbox and the MCP service
that was preventing read-only mode from being enforced for sandbox-created
MCP sessions.
The sandbox was sending x-posthog-read-only, while the MCP service was
only looking for x-posthog-readonly. Because of that mismatch, sandbox
sessions could silently miss the read-only restriction even when they were
meant to be locked down.
What changed
readonly header
the old x-posthog-read-only header
existing query-param fallback
Why
The main fix is making the sender and receiver agree on the same header
name. I also kept the MCP side backward-compatible so older internal
callers or in-flight clients don’t silently lose read-only enforcement
while everything catches up.
Validation
I verified the touched Python files compile cleanly locally. I didn’t run
the MCP vitest suite from this machine, so CI will be the final check.
Fixes #57268