[Local API Hardening] Require bearer auth for local task API - #329
Conversation
|
@dimavedenyapin Kindly review these changes |
|
@vmanvs why do we need local API hardening? |
|
@dimavedenyapin, My thinking was that the random loopback port reduces accidental exposure, but it does not authenticate the caller. The task API can mutate local task/skill state and is intended to be called by the built-in task-management MCP process. Without a shared secret, any same-host process that discovers the port could call those routes too. Examples could be a compromised package script, editor extension, browser/dev helper process, or any local process scanning So this PR is not hardening against remote network access as much as making the local trust boundary explicit: main starts the API, main gives the built-in MCP server a per-server token, and the API only accepts calls with that token. If same-user local processes are intentionally out of scope for 20x’s threat model, I’m happy to adjust/close this. I proposed it because the change is fairly small and avoids relying on random port discovery as the only guard for a state-mutating local API. |
6be0477 to
8a9dab5
Compare
|
@dimavedenyapin should I close this, or do you want to discuss this further? |
Summary
This PR adds bearer-token authentication to the local task API used by the built-in task-management MCP server.
Previously, the task API listened on
127.0.0.1with a random port and passedTASK_API_URLto the MCP process. Loopback binding and random ports reduce exposure, but they do not authenticate the caller. This PR makes the trust relationship explicit by generating a per-server token in the main process, requiring it on task API requests, and passing it only to the built-in MCP process.Related Issue
Addresses item
#2from #309.Changes
Authorization: Bearer <token>for task API requests.TASK_API_TOKENalongsideTASK_API_URLto the built-intask-managementMCP server.Testing
Checklist
pnpm typecheckpassespnpm buildsucceedspnpm test:runpassesNotes
Literal
pnpm test:runfails because the script uses Unix-style env syntax:I ran the Windows-equivalent full test command instead:
That completed, but failed in unrelated existing tests:
Failures were in:
scripts/notarize-config.test.ts: Windows path separator mismatch.src/main/agent-manager.test.ts: Windows path separator expectations, including an attachment-preview mock that checks for POSIX-style paths.src/main/worktree-manager.test.ts: Windows path separator expectations in mockedgh/gitcalls.src/main/ipc-handlers.test.ts: terminal kill mock expectation mismatch, likely because Windows terminal creation now usesnode-pty/ ConPTY rather than the old child-process path.