Repository navigation
Conversation
…onfirmation hey auth login --mcp signs hey mcp in as HEY MCP (local) (haystack#9041) on read write send: by the scoped device flow where HEY serves it (#9085), and by loopback otherwise. The credential is stored apart from the CLI's, refreshes and revokes as the local client, reads the scope HEY granted, and HEY_TOKEN never stands in for it. hey mcp serves on it alone and refuses without it. hey_send's send_draft delivers a draft after the person confirms that exact email through MCP elicitation (sending-safety D2-D6): digest-bound, single use, never retried, with the recipient caps and the exposure window, and its state in a locked, fsynced file. A host without elicitation gets the draft's link instead.
|
Revert proof on 3f8aa2f. I took out each piece, ran the tests that hold it, and restored the file from HEAD. All 16 of the 16 pieces fail their tests when they are taken out, and
|
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
Security findingsAdvisory findings (6)
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved confirmation-integrity, grant-bound exposure, and rolling recipient-limit gaps weaken the sending safeguards.
6 open issues
Reject authentication credential changes after startup · New Measure escaped content before applying the unseen-content limit · New Analyze or reject embedded HTML email bodies · New Display complete recipient addresses without truncation · New Track exposure independently of send-tool registration · New Refresh address timestamps on every claimed send · New
What changed in this PR
This PR gives hey mcp a separate, least-privilege sign-in and adds draft sending through explicit confirmation in the MCP client. Production rollout remains gated on provisioning the local MCP client ID.
Changes:
- Adds isolated MCP credentials, device-flow login with browser fallback, revocation, and status reporting.
- Introduces
hey_sendwith draft-bound confirmation, persistent send claims, recipient limits, and exposure tracking. - Updates command help, documentation, and tests for the new authentication and sending behavior.
[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or rungh pr ready --undo.
Click "Ready for review" or rungh pr readyto reengage.
| File | Description |
|---|---|
| README.md | Updates MCP setup and sending guidance. |
| internal/mcpserver/server.go | Registers sending and exposure tracking. |
| internal/mcpserver/sendstate.go | Persists claims, limits, exposure, and confirmation keys. |
| internal/mcpserver/sendprompt.go | Formats send confirmations and escapes fields. |
| internal/mcpserver/sendbody.go | Analyzes bodies, attachments, and remote references. |
| internal/mcpserver/send.go | Implements confirmed draft delivery and policy checks. |
| internal/mcpserver/send_test.go | Tests confirmation, limits, exposure, and durability. |
| internal/mcpserver/exposure.go | Classifies reads for exposure tracking. |
| internal/mcpserver/drafts.go | Updates draft-only messaging to reference confirmed sending. |
| internal/mcpserver/domains.go | Updates the threads tool description. |
| internal/mcpserver/dispatch.go | Records exposure before returning read results. |
| internal/config/mcp.go | Resolves the MCP client ID from trusted configuration. |
| internal/config/mcp_test.go | Tests client ID resolution and validation. |
| internal/cmd/sdk.go | Removes the superseded shared-client helper. |
| internal/cmd/root.go | Initializes MCP authentication separately. |
| internal/cmd/mcp.go | Uses MCP credentials and configures sending. |
| internal/cmd/mcp_test.go | Tests credential isolation, tool availability, and sending. |
| internal/cmd/mcp_auth.go | Implements MCP login, logout, and status handling. |
| internal/cmd/mcp_auth_test.go | Tests MCP authentication commands. |
| internal/cmd/help_topics.go | Documents MCP authentication environment settings. |
| internal/cmd/auth.go | Adds MCP login/logout flags and status output. |
| internal/auth/store.go | Stores scope, client, and grant metadata. |
| internal/auth/oauth.go | Captures granted scopes in token responses. |
| internal/auth/mcp.go | Defines isolated MCP authentication and scope validation. |
| internal/auth/mcp_test.go | Tests scoped login, refresh, fallback, and revocation. |
| internal/auth/device.go | Implements device authorization and polling. |
| internal/auth/auth.go | Supports separate credential keys and scoped refreshes. |
| docs/cli.md | Documents MCP authentication commands. |
| docs/agents.md | Explains MCP sign-in and confirmed sending safeguards. |
| .surface | Records MCP flags on login and logout commands. |
Review effort: Balanced
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f8aa2fc58
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Record exposure whenever the sign-in can send, including from a read-only or narrowed hey mcp, since another process on the same sign-in may send. - Refuse sends and exposing reads once the stored sign-in no longer matches the one the process started on. - Measure the body against the cap as the prompt shows it, escapes included. - Refuse attachments that aren't plain blobs: no sgid or filename, embedded content, stray attributes or markup inside. - Show every address in full in the confirmation. - Keep a new address in the rolling window for 24 hours after its last send. - Report can_send with the same predicate hey mcp serves hey_send on.
|
Revert proof on 13c80ff, after round one's fixes. I took out each piece, ran the tests that hold it, and restored the file from HEAD. All 23 of the 23 pieces fail their tests when they are taken out, and
|
|
@codex review |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved sending-safety, delivery-reporting and terminal-output issues need correction before approval.
4 open issues
6 resolved since last review
Track exposure independently of send-tool registration Display complete recipient addresses without truncation Analyze or reject embedded HTML email bodies Measure escaped content before applying the unseen-content limit Reject authentication credential changes after startup Refresh address timestamps on every claimed send
Review effort: Balanced
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 13c80ffa4f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Report a delivery HEY kept as a draft as not sent (the redirect to the edit page that mail.ConfirmDelivered reads for hey draft send). - Count a recipient known by when the mail went out, not when the draft was created. - Treat an escaped or functional style as a loading reference. - Accept an attachment's preview image only from HEY's own host, with no attributes but its size and alt text. - Refuse a send when a refresh narrowed the grant below sending. - Sanitize the device link and code before they reach the terminal, and open only a web link.
…side of the sign-in
|
Revert proof on 47dc338, after round two's fixes. I took out each piece, ran the tests that hold it, and restored the file from HEAD. All 28 of the 28 pieces fail their tests when they are taken out, and
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 47dc338afd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
🔵 Needs a closer look
Production provisioning and live verification of security-sensitive authentication and attachment delivery remain pending, alongside an unresolved mutation-acknowledgement issue.
1 open issue
1 resolved since last review
Review effort: Balanced
There was a problem hiding this comment.
🛡️ Codex Security Review · Automatically triggered
Here are some automated security review suggestions for this pull request.
Reviewed commit: 72f3cccc3a
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
…ts before the marker - On the plain path, body and subject are NFKC-normalized after invisible characters go, so a no-break space and a decomposed letter go as what they look like. The subject sent is the one the confirmation showed. - A write whose success is one exact status (rename_topic) is checked before the exposure marker, so a redirect is never reported as applied.
|
Revert proof on 0c62b58. I took out each piece, ran the tests that hold it, and restored the file from HEAD. All 45 of the 45 pieces fail their tests when they are taken out, and
|
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
🔵 Needs a closer look
Security-sensitive authentication and delivery changes need Jeremy’s final review, production client provisioning, and the documented live HEY validation before the draft hold is lifted.
No open issues 🎉
1 resolved since last review
Review effort: Balanced
|
Converged on 0c62b58, held as a draft for Jeremy. There is no auto-merge.
Findings per round: 11, 9, 7, 3, 2, 2, 1, 3, 4, 2, then 0. Two of those rounds turned out to be the same weakness found again, so I replaced the mechanism instead of adding another check:
Declined, with the reasoning in each thread:
Waiting on Jeremy:
|
* origin/main: Render nested Action Text attachment content (#335) Add hey thread comment for private thread notes (#383) Select threads with x as well as space (#359) Bump hey-sdk to v0.34.0 (#602) Update nix flake and plugin version for v1.10.1 (#600) Say why a refused Keychain read can't be fixed by signing in again (#599)
main's #599 classifies a refused Keychain read so the CLI stops asking for another login. hey mcp's own credential check still wrapped it as a plain error, though hey mcp is usually started by an MCP client's background process, the kind the Keychain refuses.
|
Merged main (6 commits, through #335) into this branch and fixed one integration gap: #599 classifies a refused Keychain read, and @codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
A custom element's name is the draft's own text, and the refusal goes back to the model without recording exposure. Name only elements HTML knows; an attribute is already named by a key from a fixed list.
govulncheck finds both reachable through html.Parse and html.Render.
|
On cf1c83a: Copilot's custom-element finding is fixed (ccd17b2), and golang.org/x/net goes to v0.62.0 for GO-2026-6866 and GO-2026-6860, which govulncheck newly reports through html.Parse and html.Render (main still pins v0.61.0, so its next Security run will fail the same way). @codex review |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
🔵 Needs a closer look
Security-sensitive OAuth, recipient checks, and rebuilt attachments require human validation against deployed HEY because server contracts are unavailable, while production MCP client provisioning remains pending.
No open issues 🎉
1 resolved since last review
Review effort: Balanced
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
Converged on cf1c83a, still held as a draft for Jeremy, with no auto-merge.
|


Held for Jeremy as a draft: don't merge, don't arm auto-merge. It can't ship before the production
HEY MCP (local)row exists, because its client id is still a TODO (below).Ship order:
bin/rails oauth:provision_local_mcp_clienton production.config.DefaultMCPClientID.hey mcpgets its own least-privilege sign-in, and it can send, with a confirmation for each email. This is the hey-cli lane of haystack #9041 (thelocal_mcprow) and #9085 (the scoped device flow). It implements sending-safety.md D2–D6 for stdio, under the roadmap's implementation requirements.What changes for a person
hey auth login --mcp # once: "HEY MCP (local)", read write send claude mcp add hey -- hey mcphey mcpexits withRun: hey auth login --mcp. That includes a person who is signed in to the CLI or hasHEY_TOKENset. This breaks existinghey mcpregistrations until the person runs the sign-in once, so it goes in the release notes.hey_send/send_draftdelivers a draft after the person confirms that exact email in their MCP client. A client that can't show a confirmation gets the draft's link instead.1.
hey auth login --mcphey/hey::mcp::<origin>, beside the CLI'shey::<origin>. With no keychain it gets its own entry incredentials.json. Its refreshes and revocations name the local client id, never the CLI's.HEY_TOKENnever stands in for it.POST /oauth/device_authorizationswithclient_idandscope=read write send, and noresource, as #9085's contract specifies. It falls back to loopback authorization with the same scope and S256 PKCE on:invalid_request:OAUTH_SCOPED_DEVICE_FLOWis off;unauthorized_client: #9041 without #9085;Other refusals, such as
invalid_client, fail and don't fall back. The poll handlesauthorization_pending,slow_down(+5 s),access_denied,expired_tokenandinvalid_scope, and sendsinstall_id. The device code is never printed. The link and user code are sanitized before they're shown, and only an http(s) link is opened.scope.hey mcpserves nohey_send. If write is missing, it serves reads only.scopemeans exactly what was requested (RFC 6749 §5.1).full, which would mean the configured id is the CLI's own row. An id equal to the CLI's is refused before HEY is contacted.hey auth logout --mcprevokes the grant withDELETE /oauth/tokens, sending the local client id and the refresh token, then removes the local credential. If the revocation fails, the credential is still removed, and a warning says to remove the app in Connected Apps.hey auth statusreports both credentials, in JSON undermcpand styled as anMCP:line. Neither form ever includes a token.HEY_MCP_CLIENT_ID, then frommcp_client_idsin the global config, keyed by origin, then fromconfig.DefaultMCPClientID."", a marked TODO, until Jeremy provisions production withbin/rails oauth:provision_local_mcp_client. Staging and dev then get the same id withCLIENT_ID=, so one constant serves every environment..hey/config.jsoncan't set the id.2. No fallback to the CLI credential
hey mcpnever falls back, not even for reads. The reasons:full.--read-onlyishey mcp's own client-side rule, not HEY's. A read-only fallback still puts a full-scope token in the process the model drives. Then the server-enforced containment that justifies thelocal_mcprow (#9041's threat model) would rest on hey-cli's gateway filter staying correct. That filter is the long-tail code the row exists to contain.3.
send_draft, written in hey-cliWhy it isn't imported.
send_draft. Itsserverpackage is still underinternal/, and the move out is hey-mcp-server#14, which is open.go.modhas no hey-mcp-server dependency.So the code is in
internal/mcpserver, in files the W5.1 move can lift as a unit (send.go,sendstate.go,sendbody.go,sendprompt.go,exposure.go).How it's wired.
hey_sendis a separate tool (D7) withdestructiveHintandopenWorldHinttrue. It is served only when all of these hold:readandsend(expanded, sowritecounts asread);--read-only;GetMessageEdit, applies the policy, and returns an elicitation. Its HMAC-signed request state carries the draft's fingerprint, the sign-in, the origin, a 5-minute expiry and a method marker.acceptwithconfirm: true. It then re-fetches the draft and recomputes the fingerprint. Any change fails with "the draft changed since you approved it". It re-runs the policy, then claims the draft and charges the caps in one locked update. Last, it callsSendDraftonce, through a client that neither retries nor refreshes-and-resends after a 401. The token is refreshed just before the send instead. An answer with an id and no thread, where the draft's edit page is still served, is reported as not sent, asmail.ConfirmDeliveredreads it. Every delivered field is written from the verified snapshot, and HEY replaces the whole recipient set whenaddressedis present (Entry::Addressed#address).elicitation/createfrom the SDK shim. Both are tested."Name" <address>. Controls, line separators and every invisible character (all Unicode format characters, which include bidi, zero-width and tag characters, plus variation selectors and private-use characters) are escaped as\u{…}. Header fields are single-line. Delivered ones (the subject, attachment names and captions) are shown whole, and only display names are capped. The body is indented, and the body shows at most 1,500 characters with a link to the rest. New recipients are marked(new: never emailed before).send_draftreturns a non-errorsent: falseresult with the draft's edit link. A decline or cancel gets the same result. There is no "I confirm" parameter.Single use and never retried. The claim key is
origin#draft_idand nothing else. It lasts 24 hours and is never released, even when HEY refuses. A replayed state, a second confirmation of the same draft, and a resend after a failure are all refused, and in each caseSendDraftis called once.D4.
GrantedAt. It is looked up through advanced search (refine[to]), and each match is verified locally against the entry's creator (one of the identity's addresses), its delivery time (active_at, notcreated_at, so a draft made before the sign-in and sent after it doesn't count) and its recipients. So a loose search can't make an address known. A failed lookup counts as new. Contact records never count.GrantKey, minted at login and stored with the credential.D2's unseen-content and remote-reference rules.
<br>), with trailing whitespace and invisible characters dropped. So no markup choice (hidden text, a list's type, bold against italic, a break against a paragraph) can carry something the person didn't read. Such a draft must fit the 1,500-character prompt whole and have no attachment. The plain/as-is decision is signed into the confirmation, and a change between rounds refuses the send. To known recipients on an unexposed sign-in, the draft goes as it is. This replaced a lossless-markup classifier whose every edge was a new hidden channel.img,iframe,link,style,svgand the like,src/srcset/backgroundand similar attributes, and anystylewith an escape or a(, which Trix never writes.content(Trix's filename-less embedded HTML bodies), is refused for every recipient. Unverified against a live HEY: thatUpdateMessageaccepts the rebuilt, childless figure as Trix's own submission.noscript,templateand the raw-text fallbacks are refused as loading markup.send_atisn't implemented.D6 (exposure).
screened_inisn't in the hey-sdk model (still absent in 0.34.0, which main now pins). SoreadExposesclassifies every served read by what it can return.reveal_contactreturns the contact).--read-onlyor narrowedhey mcp, since another process on the same sign-in may send.hey auth login --mcpis refused rather than adopted. A refresh that narrows the grant below sending refuses sends and exposing reads until the process restarts.State on stdio (the roadmap's invariants row) is
$XDG_STATE_HOME/hey-cli/mcp-send-state.json. It holds claims, send times, new-address membership and exposure.flockon a separatemcp-send-state.lockthat is never renamed, with a 5-second timeout that fails closed.mcp-confirm.key, is generated once per installation under the same lock.Help and docs.
hey mcp --help,hey auth login/logout --help,docs/agents.md("Its own sign-in", "Drafts first, sends confirmed"),docs/cli.md, the environment help topic (HEY_MCP_CLIENT_ID), and the README's one MCP line, whose example no longer ran without the new sign-in..surfacegains--mcpon login and logout.Tests
make checkis green: fmt-check, vet, golangci-lint, the unit tests, sync-skills and tidy-check. The new and changed tests cover the brief's six cases and more:TestMCPLoginStoresACredentialApartFromTheCLIs(two keychain items, the CLI's untouched),…InThePlaintextFallbackToo,TestAuthLoginMCPKeepsTheCLIsCredentialAndStatusShowsBoth(through the command, with status and logout)hey mcprefuses with no MCP credentialTestMCPCommandRefusesWithoutItsOwnCredential(CLI signed in andHEY_TOKENset; nothing reaches HEY),TestMCPManagerNeverUsesHEYTokenTestMCPLoginHonorsTheScopeHEYGranted(all, send unchecked, omitted),TestMCPLoginRefusesAScopeItDidntAskFor(full),TestMCPCommandServesWhatTheGrantAllows,TestMCPRefreshNamesTheLocalClientAndKeepsTheScopesend_draftneeds a confirmed elicitationTestSendDraftSendsOnlyWhatThePersonConfirmed(both protocol eras),TestSendDraftWithoutApprovalSendsNothing(decline, cancel, unticked)TestSendDraftRefusesADraftChangedAfterApproval(Bcc added, recipient swapped, subject, body, sender, schedule),TestAnAttachmentReplacedWithTheSameNameAndSizeIsAChange,TestAReplayedConfirmationSendsNothing,TestTwoConfirmationsOfOneDraftSendItOnce,TestSendDraftSendsADraftOnce,TestAForgedOrExpiredConfirmationSendsNothingTestSendDraftCantSendFromAHostWithoutElicitation(both eras)Also covered: caps (
TestSendDraftCapsRecipientsSendsAndNewAddresses), known recipients (TestSendDraftFlagsANewRecipient,TestMailSentAfterTheSignInDoesntMakeARecipientKnown), exposure (TestAnUnscreenedReadRefusesSendsToNewRecipients,TestABoxReadDoesntExpose,…BetweenPromptAndApproval…,TestTheClaimRereadsExposure,TestAnExposedReadIsRefusedWhenItsMarkerCantBeWritten,TestEveryServedReadIsClassifiedForExposure), unseen content and remote references, inert rendering, durability (TestAFailedFsyncFailsTheSendClosed,TestEitherFailedFsyncFailsTheClaim,TestClaimsAndCapsSurviveARestart), caps charged in the claim itself (TestTheClaimEnforcesTheCaps), client id resolution (TestMCPClientIDResolution), never retried (TestAFailedSendIsNeverRetriedAndStaysClaimed), and an end-to-end run throughhey mcp(TestMCPCommandSendsAConfirmedDraftOnce).Revert proof: in the latest revert-proof comment: 45 of 45 pieces fail their tests when taken out.
Not verified against a live HEY
addressed,creatorandcreated_at. If HEY's summaries leaveaddressedout, every recipient reads as new. That fails closed, but it's strict: no unseen content goes anywhere, and the 10-new-address cap applies to everyone.GetMessageEditfetches. If the sgid isn't stable, a draft with an attachment always fails as "changed". That also fails closed.What haystack and hey-mcp-server still need
HEY MCP (local)in production, then setconfig.DefaultMCPClientID, then staging and dev withCLIENT_ID=.OAUTH_SCOPED_DEVICE_FLOWper host (until then the login falls back to loopback).screened_inin the SDK model (GetTopic, haystack#9083), so the exposure table can read screening per response rather than per operation.hey mcpimports them.UpdateMessageidempotent with retries (D2.4).send_draftgoes through hey-cli's no-retry client, as everyhey mcpmutation does.