Keep the static bearer token, and make it hold its weight (#114) - #129
Merged
Conversation
Decision recorded: the HTTP transport authenticates with a static bearer token, not OAuth. MCP authorization is OPTIONAL and explicitly SHOULD NOT be used on stdio, so this only ever concerned the HTTP transport - and becoming a real OAuth resource server would mean an authorization server we would have to stand up, a JWT stack in a 32-bit net472 binary, and a STABLE public hostname, because OAuth binds a token to the servers canonical URI and publishes metadata beneath it. A tunnel name that changes on every restart breaks both. One bench, one bus, a tunnel opened deliberately: a shared secret is the right size. Three fixes that stand on their own merits: 1. A non-loopback bind with no token now REFUSES TO START. It used to log a warning and serve anyway. What is behind this endpoint is physical control of instruments, the remedy is one environment variable, and a warning in stderr is not a control. Loopback with no token still serves - that is the local development case. 2. The token comparison no longer returns early on the first differing byte. Over a tunnel the timing attack is poor, but a comparison that does not leak how much of the token was right costs nothing. 3. The Origin guard and tunnels interact in a way that was never written down: a server-side caller sends no Origin and passes (which is how the cloud connectors reach us), while a BROWSER-based client on the tunnels own domain sends one and is rejected. Correct as a rebinding defence - now documented as deliberate, with an origin allow-list named as the fix if one is ever wanted. The ChatGPT doc stopped recommending an unauthenticated public tunnel, which was the genuinely dangerous advice in the repo: it now says plainly that ChatGPT is unsupported, why, and what to do instead (Copilot, or authentication at the edge, which buys the expiry and revocation a static token cannot). Also corrected a warning that only fired for a non-loopback bind: the loopback case is the one people actually tunnel, so that is where the reminder belongs. 7 tests.
12 tasks
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.
Closes #114. Decision: Option A — the HTTP transport authenticates with a static bearer token, not OAuth. Full reasoning is in the analysis comment; this PR records the decision and closes the gaps that made the current implementation weaker than a shared secret needs to be.
Why A
MCP authorization is OPTIONAL and explicitly SHOULD NOT be used on stdio — so this only ever concerned the HTTP transport (Copilot, ChatGPT), never Claude Desktop or the
.mcpbbundle. Becoming a real OAuth resource server means an authorization server we'd have to stand up, a JWT stack in a 32-bit net472 binary, and — the part that actually blocks it — a stable public hostname, because OAuth binds a token to the server's canonical URI and publishes metadata beneath it. A tunnel name that changes on every restart breaks both. One bench, one bus, a tunnel opened deliberately: a shared secret is the right size.Three fixes that stand on their own merits
1. A non-loopback bind with no token now refuses to start. It used to log a warning and serve anyway. Behind this endpoint is physical control of instruments, the remedy is one environment variable, and a warning in stderr is not a control. Loopback with no token still serves — that's the local development case.
2. The token comparison no longer returns early on the first differing byte. Over a tunnel the timing attack is poor, but a comparison that doesn't leak how much of the token was right costs nothing.
3. The Origin guard and tunnels interact in a way that was never written down. A server-side caller sends no
Originand passes — which is how the cloud connectors reach us — while a browser-based client on the tunnel's own domain sends one and is rejected. Correct as a DNS-rebinding defence; now documented as deliberate, with an explicit origin allow-list named as the fix if a browser client is ever wanted.The documentation change that mattered most
packaging/chatgptwas recommending running with no token behind a public tunnel as the "testing" path. That was the genuinely dangerous advice in the repo — an unauthenticated public URL onto a live instrument bus. It now says plainly that ChatGPT is unsupported, why, and what to do instead: use Copilot, or put authentication at the edge (Cloudflare Access or similar), which buys the expiry and revocation a static token cannot.I also moved the "no token configured" warning: it only fired for a non-loopback bind, but loopback is the case people actually tunnel, so that is where the reminder belongs. Binding loopback is not protection once the port is tunnelled, and the server cannot see that it has been.
Verification
README,
packaging/templates/chatgpt.README.md.tmplandStart-GpibMcpHttp.ps1all updated so the decision is documented rather than rediscovered.