feat(mcp): add one-shot auth descriptor handoff

[arman-instalily]

Summary

• accept MCP OAuth credentials from an inherited, one-shot file descriptor
• allow per-invocation MCP URL and channel-type overrides without rewriting archive config
• document the secure parent-process workflow and complete shell flags

The descriptor is consumed and closed, bounded to 64 KiB, and takes precedence over ambient connector environment variables. This keeps short-lived user tokens out of argv, environment variables, and named files while preserving Slacrawl's existing incremental upserts and thread repair.

Validation

• GOWORK=off go test ./internal/slackmcp ./internal/cli
• GOWORK=off go test -count=1 ./...
• make check passed module verification, formatting, vet, vulnerability scan, dead-code analysis, tests, and CLI smoke; the final release snapshot step could not run locally because goreleaser is not installed

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

[clawsweeper]

Codex review: needs real behavior proof before merge. Reviewed August 17, 2026, 5:53 PM ET / 21:53 UTC.

ClawSweeper review

What this changes

Adds one-shot MCP OAuth credentials from an inherited descriptor plus temporary MCP URL and channel-type overrides for sync.

Merge readiness

⛔ Blocked until real behavior proof from a real setup is added - 9 items remain

Keep open: the PR still sends a one-shot bearer credential to any endpoint supplied through --mcp-url, and it lacks real after-fix behavior proof.

Priority: P1
Reviewed head: 6a41db4c2eeb11ada219053c655895d08465a5bd
Owner decision: Required. See Decision needed.

Review scores

Measure	Result	What it means
Overall readiness	🧂 unranked krab (1/6)	The unresolved credential-routing defect and missing real behavior proof prevent merge readiness.
Proof confidence	🧂 unranked krab (1/6)	Needs real behavior proof before merge: The PR reports automated tests and checks but no redacted after-fix terminal output, logs, or transport trace showing descriptor consumption and successful MCP authentication. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality	🦪 silver shellfish (2/6)	Security review found an item that needs attention.

Verification

Check	Result	Evidence
Real behavior	Needs proof	Needs real behavior proof before merge: The PR reports automated tests and checks but no redacted after-fix terminal output, logs, or transport trace showing descriptor consumption and successful MCP authentication. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed	5 items	Credential-routing path: The branch accepts a descriptor credential and an arbitrary MCP URL in the same sync invocation. Bearer transmission: The shared MCP client places its access token in the Authorization header of requests to its configured endpoint. Documented combination: The PR documentation explicitly demonstrates descriptor credentials together with an overridden endpoint.
Findings	1 actionable finding	[P1] Restrict descriptor credentials to trusted endpoints
Security	Needs attention	Descriptor token can be routed to an arbitrary endpoint: A descriptor token takes precedence over ambient credentials while the same invocation can set the MCP URL; the HTTP client sends that token in Authorization requests to the selected URL.

How this fits together

The sync command resolves MCP connector settings and OAuth credentials, then supplies them to the HTTP MCP client that retrieves Slack data into the local archive. These new flags alter that credential and endpoint selection for one invocation.

flowchart LR
Parent[Trusted parent process] --> CLI[Sync command]
CLI --> Overrides[Temporary MCP settings]
Overrides --> Auth[Credential resolution]
Auth --> Client[HTTP MCP client]
Client --> Archive[Slack archive sync]

Loading

Decision needed

Question	Recommendation
Should descriptor credentials be restricted to the configured or allowlisted HTTPS MCP endpoint, rather than being usable with arbitrary --mcp-url values?	Restrict descriptor endpoint use: Reject arbitrary URL overrides while descriptor credentials are present, or permit only a narrowly approved HTTPS endpoint set.

Why: This determines the permanent bearer-token audience boundary and needs maintainer security intent.

Before merge

• Add real behavior proof - Needs real behavior proof before merge: The PR reports automated tests and checks but no redacted after-fix terminal output, logs, or transport trace showing descriptor consumption and successful MCP authentication. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
• Restrict descriptor credentials to trusted endpoints (P1) - --mcp-auth-fd makes the descriptor token authoritative, while these lines replace the MCP endpoint for the same invocation. Because the HTTP client sends that token as a bearer header, a caller can direct it to an arbitrary host; reject this combination or enforce an approved HTTPS endpoint/audience policy. This is the still-unfixed prior blocker.
• Resolve security concern: Descriptor token can be routed to an arbitrary endpoint - A descriptor token takes precedence over ambient credentials while the same invocation can set the MCP URL; the HTTP client sends that token in Authorization requests to the selected URL.
• Resolve merge risk (P1) - A caller-controlled --mcp-url can receive the bearer token read from --mcp-auth-fd, including over an untrusted or non-HTTPS endpoint.
• Resolve merge risk (P1) - The PR provides test/check claims only, not a redacted after-fix trace of descriptor consumption and authenticated MCP sync.
• Complete next step (P2) - A maintainer must choose the token-to-endpoint trust policy before a narrow repair can be finalized.
• Improve patch quality - Bind descriptor credentials to approved HTTPS endpoint(s).
• Improve patch quality - Add a redacted real MCP run or transport trace showing one-shot descriptor consumption and successful authentication, then update the PR body for re-review.

Findings

• [P1] Restrict descriptor credentials to trusted endpoints — internal/cli/app.go:522-524
• [high] Descriptor token can be routed to an arbitrary endpoint — internal/cli/app.go:522

Agent review details

Security

Needs attention: The PR combines bearer-token handoff with caller-selectable HTTP endpoint routing.

Review metrics

Metric	Value	Why it matters
Changed surface	9 files; production +56, tests +35, docs/spec +30	The feature crosses CLI parsing, credential resolution, completions, tests, and operator documentation.

Merge-risk options

Maintainer options:

1. Constrain descriptor endpoint use (recommended)
   Reject or allowlist endpoint overrides when a descriptor credential is supplied, and cover accepted and rejected combinations.
2. Accept caller-controlled endpoint routing
   Explicitly approve the trusted-parent audience contract and require redacted proof that only audience-matched credentials are provided.

Technical review

Best possible solution:

Keep descriptor-backed credentials bound to an approved HTTPS MCP endpoint, then provide a redacted local trace proving one-shot consumption and a successful authenticated sync.

Do we have a high-confidence way to reproduce the issue?

Yes, from source: invoke sync with both descriptor and URL flags; credential resolution prefers the descriptor and the MCP client sends it as a bearer header to that URL. A live transport trace has not been provided.

Is this the best way to solve the issue?

No; the current approach needs an enforceable endpoint/audience boundary before pairing a descriptor token with an endpoint override is safe.

Full review comments:

• [P1] Restrict descriptor credentials to trusted endpoints — internal/cli/app.go:522-524
--mcp-auth-fd makes the descriptor token authoritative, while these lines replace the MCP endpoint for the same invocation. Because the HTTP client sends that token as a bearer header, a caller can direct it to an arbitrary host; reject this combination or enforce an approved HTTPS endpoint/audience policy. This is the still-unfixed prior blocker.
  Confidence: 0.98

Overall correctness: patch is incorrect
Overall confidence: 0.98

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning high; reviewed against 526888485c56.

Labels

Label justifications:

• P1: The patch can expose a short-lived OAuth bearer token during an active MCP sync.
• merge-risk: 🚨 auth-provider: It changes OAuth credential precedence and the endpoint used to authenticate MCP requests.
• merge-risk: 🚨 security-boundary: An inherited bearer credential can be combined with a caller-selected HTTP endpoint.
• rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦪 silver shellfish.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR reports automated tests and checks but no redacted after-fix terminal output, logs, or transport trace showing descriptor consumption and successful MCP authentication. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

Security concerns:

• [high] Descriptor token can be routed to an arbitrary endpoint — internal/cli/app.go:522
  A descriptor token takes precedence over ambient credentials while the same invocation can set the MCP URL; the HTTP client sends that token in Authorization requests to the selected URL.
  Confidence: 0.98

What I checked:

• Credential-routing path: The branch accepts a descriptor credential and an arbitrary MCP URL in the same sync invocation. (internal/cli/app.go:516, 6a41db4c2eeb)
• Bearer transmission: The shared MCP client places its access token in the Authorization header of requests to its configured endpoint. (internal/mcpclient/client.go:199, 526888485c56)
• Documented combination: The PR documentation explicitly demonstrates descriptor credentials together with an overridden endpoint. (docs/configuration.md:172, 6a41db4c2eeb)
• Current-main comparison: Current main lacks the descriptor reader and per-invocation MCP URL flags, so the requested feature is not already implemented. (internal/slackmcp/client.go:354, 526888485c56)
• History ownership signal: Available shallow-history blame traces the current-main MCP path to the v0.8.5 release baseline; deeper feature provenance is unavailable locally. (internal/slackmcp/client.go:354, b793612bfa8a)

Likely related people:

• Peter Steinberger: The available current-main blame/history trail reaches the v0.8.5 baseline containing the MCP connector path; deeper history is unavailable in this checkout. (role: recent release-baseline contributor; confidence: low; commits: b793612bfa8a; files: internal/slackmcp/client.go, internal/cli/app.go)

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (1 earlier review cycle)

• reviewed 2026-08-17T21:43:34.575Z sha d744e23 :: needs real behavior proof before merge. :: [P1] Bind descriptor credentials to a trusted endpoint

[steipete]

Maintainer triage: NEEDS-PETER. Recommend CLOSE in its current form unless Peter explicitly wants this new credential-handoff API. Left open; no implementation changes made.

The code introduces three CLI flags, gives an inherited descriptor precedence over ambient MCP credentials, and lets the same invocation replace the HTTP endpoint. This is a new auth/API contract, not a repair to existing behavior. The descriptor read is bounded to 64 KiB and closed after consumption, but the token-to-endpoint trust contract remains a product/security decision. A trusted parent intentionally choosing both values is not by itself proof of an exploit; the question is whether endpoint control and credential selection are expected to cross a trust boundary.

Decision needed: approve or decline the descriptor workflow; if approved, define whether the parent may choose arbitrary endpoints or credentials must be bound to configured/approved HTTPS destinations, and define credential-read cancellation behavior when the parent never closes the pipe. Then validate a real authenticated MCP sync from the intended parent, with secrets redacted, including successful one-shot consumption and failure/cancellation cases. The submitted tests do not provide that integration proof. No owner approval was found in the comments.