Skip to content

[Local API Hardening] Require bearer auth for local task API - #329

Open
vmanvs wants to merge 1 commit into
peakflo:mainfrom
vmanvs:fix/task-api-bearer-token
Open

[Local API Hardening] Require bearer auth for local task API#329
vmanvs wants to merge 1 commit into
peakflo:mainfrom
vmanvs:fix/task-api-bearer-token

Conversation

@vmanvs

@vmanvs vmanvs commented May 31, 2026

Copy link
Copy Markdown
Contributor

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.1 with a random port and passed TASK_API_URL to 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 #2 from #309.

Changes

  • Generate a high-entropy task API auth token when the task API server starts.
  • Require Authorization: Bearer <token> for task API requests.
  • Use timing-safe token comparison for bearer-token validation.
  • Pass TASK_API_TOKEN alongside TASK_API_URL to the built-in task-management MCP server.
  • Update the task-management MCP server to include the bearer token on task API requests.
  • Add regression tests for missing, invalid, and valid task API tokens.
  • Add coverage ensuring the built-in task-management MCP server receives both the task API URL and auth token.

Testing

  • Focused tests pass:
$env:ELECTRON_RUN_AS_NODE='1'
.\node_modules\.bin\electron.cmd .\node_modules\vitest\vitest.mjs --project main src/main/task-api-server.test.ts

$env:ELECTRON_RUN_AS_NODE='1'
.\node_modules\.bin\electron.cmd .\node_modules\vitest\vitest.mjs --project main src/main/agent-manager.test.ts -t "injects task API URL and bearer token"

$env:ELECTRON_RUN_AS_NODE='1'
.\node_modules\.bin\electron.cmd .\node_modules\vitest\vitest.mjs --project main src/main/mcp-servers/task-management-mcp-schema.test.ts

Checklist

  • pnpm typecheck passes
  • pnpm build succeeds
  • pnpm test:run passes
  • No hardcoded color classes (use CSS variable tokens)

Notes

Literal pnpm test:run fails because the script uses Unix-style env syntax:

'ELECTRON_RUN_AS_NODE' is not recognized as an internal or external command

I ran the Windows-equivalent full test command instead:

$env:ELECTRON_RUN_AS_NODE='1'
.\node_modules\.bin\electron.cmd .\node_modules\vitest\vitest.mjs run

That completed, but failed in unrelated existing tests:

Test Files  4 failed | 91 passed (95)
Tests       12 failed | 1348 passed (1360)

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 mocked gh/git calls.
  • src/main/ipc-handlers.test.ts: terminal kill mock expectation mismatch, likely because Windows terminal creation now uses node-pty / ConPTY rather than the old child-process path.

@vmanvs

vmanvs commented May 31, 2026

Copy link
Copy Markdown
Contributor Author

@dimavedenyapin Kindly review these changes

@dimavedenyapin

Copy link
Copy Markdown
Contributor

@vmanvs why do we need local API hardening?

@vmanvs

vmanvs commented May 31, 2026

Copy link
Copy Markdown
Contributor Author

@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 127.0.0.1 ports under the user session.

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.

@vmanvs
vmanvs force-pushed the fix/task-api-bearer-token branch from 6be0477 to 8a9dab5 Compare June 1, 2026 07:35
@vmanvs

vmanvs commented Jun 2, 2026

Copy link
Copy Markdown
Contributor Author

@dimavedenyapin should I close this, or do you want to discuss this further?

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.

2 participants