feat(auth): accept a Stalwart app password over Basic, opt-in #25

Merged
jlxq0 merged 1 commit from feat/stalwart-app-password into main 2026-08-29 12:10:15 +00:00
Owner

Closes the case in #24: a second identity needed a mount and only a Logto JWT authenticated, so every new identity cost the principal a browser OAuth.

The shape

The two paths are distinguished by the request's own scheme, not by a fallback. Authorization: Bearer stays Logto-JWT-only, so a garbage bearer takes exactly the path it took before this existed and cannot reach Stalwart by failing the JWT check first. Authorization: Basic is the static path. There is no if the JWT fails, try something else branch, which is how a validated credential and an unvalidated one become the same object.

Acceptance is Stalwart's answer, never a shape check, which is what keeps this from becoming hevy-mcp. Nothing is forwarded on the strength of being non-empty.

Off by default behind JMAP_MCP_ALLOW_STALWART_APP_PASSWORD. An app password is long-lived and validated by the mail server rather than by Logto, so enabling it is a deployment decision about that mailbox.

The server says which credential it holds rather than leaving it inferred: AccessToken carries its scheme, /token/introspect reports auth_method, and every outbound request builds its header from header_value() so no call site can pick a scheme.

The predicate, and why it is not 200

Stalwart answers a request with no Authorization header with 200, and that body parses cleanly into JmapSession:

no Authorization header   200, 1297-byte capabilities document
garbage Basic             401
garbage Bearer            401

primaryAccounts {}, username "". So a fetch that dropped the credential returns Ok and the authentication step succeeds. The predicate is is_authenticated: 200 and a non-empty username or a real entry in primaryAccounts. Recorded in AGENTS.md, because it is the first predicate anyone reaches for.

Mutations, each red on its own alone

mutation red
is_authenticated always true the two session tests
drop the opt-in guard basic_is_ignored_unless_the_deployment_opted_in
header_value drops the scheme scheme_travels_with_the_secret
route a Bearer to the static path a_garbage_bearer_still_goes_to_the_jwt_path

One methodology note. My first attempt at the is_authenticated mutation reported all 177 green, which reads exactly like a test that pins nothing. It had matched nothing: rustfmt had reflowed the expression. Applying it through a script that asserts its own anchor showed two tests red. An unapplied mutation and a worthless test produce the same output.

rejects_basic_scheme is kept rather than deleted. The contract it records did not change, only what happens to a Basic header elsewhere.

Gates and review

Five on 1.93.0: fmt, clippy -D warnings, test (177), audit, deny.

Codex, asked whether any request can authenticate that would not have before, other than by presenting an app password Stalwart accepts: None.

Acceptance

The half that is mine, a garbage bearer still returning 401, runs after deploy. The positive half needs lucy@lindner.earth's app password, which I do not hold and will not extract.

Refs #24

Closes the case in #24: a second identity needed a mount and only a Logto JWT authenticated, so every new identity cost the principal a browser OAuth. ## The shape **The two paths are distinguished by the request's own scheme, not by a fallback.** `Authorization: Bearer` stays Logto-JWT-only, so a garbage bearer takes exactly the path it took before this existed and cannot reach Stalwart by failing the JWT check first. `Authorization: Basic` is the static path. There is no *if the JWT fails, try something else* branch, which is how a validated credential and an unvalidated one become the same object. **Acceptance is Stalwart's answer, never a shape check**, which is what keeps this from becoming `hevy-mcp`. Nothing is forwarded on the strength of being non-empty. **Off by default** behind `JMAP_MCP_ALLOW_STALWART_APP_PASSWORD`. An app password is long-lived and validated by the mail server rather than by Logto, so enabling it is a deployment decision about that mailbox. **The server says which credential it holds** rather than leaving it inferred: `AccessToken` carries its scheme, `/token/introspect` reports `auth_method`, and every outbound request builds its header from `header_value()` so no call site can pick a scheme. ## The predicate, and why it is not `200` Stalwart answers a request with **no** `Authorization` header with **200**, and that body parses cleanly into `JmapSession`: ``` no Authorization header 200, 1297-byte capabilities document garbage Basic 401 garbage Bearer 401 ``` `primaryAccounts` `{}`, `username` `""`. So a fetch that dropped the credential returns `Ok` and **the authentication step succeeds**. The predicate is `is_authenticated`: 200 **and** a non-empty `username` or a real entry in `primaryAccounts`. Recorded in `AGENTS.md`, because it is the first predicate anyone reaches for. ## Mutations, each red on its own alone | mutation | red | |---|---| | `is_authenticated` always `true` | the two session tests | | drop the opt-in guard | `basic_is_ignored_unless_the_deployment_opted_in` | | `header_value` drops the scheme | `scheme_travels_with_the_secret` | | route a `Bearer` to the static path | `a_garbage_bearer_still_goes_to_the_jwt_path` | **One methodology note.** My first attempt at the `is_authenticated` mutation reported all 177 green, which reads exactly like a test that pins nothing. It had matched nothing: rustfmt had reflowed the expression. Applying it through a script that asserts its own anchor showed two tests red. **An unapplied mutation and a worthless test produce the same output.** `rejects_basic_scheme` is kept rather than deleted. The contract it records did not change, only what happens to a `Basic` header elsewhere. ## Gates and review Five on 1.93.0: `fmt`, `clippy -D warnings`, `test` (177), `audit`, `deny`. Codex, asked whether any request can authenticate that would not have before, other than by presenting an app password Stalwart accepts: **`None`**. ## Acceptance The half that is mine, a garbage bearer still returning 401, runs after deploy. **The positive half needs `lucy@lindner.earth`'s app password, which I do not hold and will not extract.** Refs #24
feat(auth): accept a Stalwart app password over Basic, opt-in
All checks were successful
CI / tag-ancestry (pull_request) Successful in 3s
CI / cargo (pull_request) Successful in 2m7s
CI / docker (pull_request) Successful in 51s
636ff2c324
Closes the case in #24: a second identity needed a mount and only a Logto JWT
authenticated, so every new identity cost the principal a browser OAuth.

The two credential paths are distinguished by the request's own scheme rather
than by a fallback. Authorization: Bearer stays Logto-JWT-only, so a garbage
bearer takes exactly the path it took before this existed and cannot reach
Stalwart by failing the JWT check first. Authorization: Basic is the static
path. There is no "if the JWT fails, try something else" branch, which is how a
validated credential and an unvalidated one become the same object.

Acceptance is Stalwart's answer, never a shape check, so this does not become
hevy-mcp: nothing is forwarded on the strength of being non-empty.

Off by default behind JMAP_MCP_ALLOW_STALWART_APP_PASSWORD. Until now this
server had only ever held a credential Logto validated; an app password is
long-lived and checked by the mail server instead, so enabling it is a
deployment decision about that mailbox.

The predicate is is_authenticated, not a 200. Stalwart returns 200 with a
capabilities-only document when NO Authorization header is sent, and that body
deserialises into JmapSession without error, so a fetch that dropped the
credential would authenticate. Only a bad credential gives 401. Measured, and
recorded in AGENTS.md because it is the first predicate anyone reaches for.

AccessToken now carries its scheme and every outbound request builds its header
from header_value(), so no call site can choose a scheme and an app password
can never go out as Bearer. /token/introspect reports auth_method, so the
server says which credential it is holding rather than leaving it to be
inferred.

Tests, each mutation-checked and each red on its own mutation alone:
  is_authenticated always true        -> the two session tests
  drop the opt-in guard               -> basic_is_ignored_unless_opted_in
  header_value drops the scheme       -> scheme_travels_with_the_secret
  route a Bearer to the static path   -> a_garbage_bearer_still_goes_to_jwt

rejects_basic_scheme is kept rather than deleted: the contract it records did
not change, only what happens to a Basic header elsewhere.

Five gates green on 1.93.0: fmt, clippy -D warnings, test (177), audit, deny.

Refs #24

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Author
Owner

Merging on the code. fmt 0, test 0 with 177 passed, CI green on cargo, docker and tag-ancestry.

The one red is a clippy that origin/main fails identically:

error: unused `async` for async trait impl function with no `.await` statements
  --> src/mcp.rs:1712  #[tool_handler(router = self.tool_router)]

local clippy 0.1.98 (88d9e12ae1 2026-08-18)   101 on this head AND on main
CI on both shas                                cargo success

So local and CI disagree about the same commits, and the disagreement is the toolchain. ci.yml uses dtolnay/rust-toolchain@<sha> # stable, which is a moving target: the runner's cached stable and mine are the same word and not the same compiler. A local gate can neither confirm nor contradict CI while that holds.

withings-mcp hit this lint two days ago: #[allow(clippy::unused_async_trait_impl)] above #[tool_handler], because it fires on macro-generated methods and there is nothing to rewrite. An #[allow] naming a lint the running clippy does not have is itself an error under -D warnings, so adding it sets a floor and ci.yml must then name a version rather than stable. Its own change, with the floor stated, rather than smuggled into this one.

What this unblocks and what it does not

Lucy gets a route to a mount that authenticates as herself, which she has not had, and without which she has no legitimate access to that mailbox at all.

The positive half of the acceptance is unrun. She can authenticate as herself is a claim about code until her app password runs against a deployment with the flag on. That has been outstanding since 2026-08-28 and it is mine to have run rather than yours.

And this does not close the requirement it looks like it closes. owned_addresses appends a global Arc<Vec<String>> to every authenticated caller, so with this merged she authenticates as herself and is still handed another account's address as a sendable From. Scoping that to an owning account is a narrowing of what is deployed today, it is the next change, and it wants to land before any send tool is granted again.

**Merging on the code. fmt 0, test 0 with 177 passed, CI green on `cargo`, `docker` and `tag-ancestry`.** **The one red is a clippy that `origin/main` fails identically:** error: unused `async` for async trait impl function with no `.await` statements --> src/mcp.rs:1712 #[tool_handler(router = self.tool_router)] local clippy 0.1.98 (88d9e12ae1 2026-08-18) 101 on this head AND on main CI on both shas cargo success **So local and CI disagree about the same commits, and the disagreement is the toolchain.** `ci.yml` uses `dtolnay/rust-toolchain@<sha> # stable`, which is a moving target: the runner's cached stable and mine are the same word and not the same compiler. **A local gate can neither confirm nor contradict CI while that holds.** **`withings-mcp` hit this lint two days ago**: `#[allow(clippy::unused_async_trait_impl)]` above `#[tool_handler]`, because it fires on macro-generated methods and there is nothing to rewrite. **An `#[allow]` naming a lint the running clippy does not have is itself an error under `-D warnings`**, so adding it sets a floor and `ci.yml` must then name a version rather than `stable`. **Its own change, with the floor stated, rather than smuggled into this one.** ## What this unblocks and what it does not **Lucy gets a route to a mount that authenticates as herself**, which she has not had, and without which she has no legitimate access to that mailbox at all. **The positive half of the acceptance is unrun.** *She can authenticate as herself* is a claim about code until her app password runs against a deployment with the flag on. **That has been outstanding since 2026-08-28 and it is mine to have run rather than yours.** **And this does not close the requirement it looks like it closes.** `owned_addresses` appends a global `Arc<Vec<String>>` to every authenticated caller, so with this merged she authenticates as herself **and is still handed another account's address as a sendable `From`.** Scoping that to an owning account is a narrowing of what is deployed today, it is the next change, and it wants to land before any send tool is granted again.
jlxq0 merged commit 468d78a9e4 into main 2026-08-29 12:10:15 +00:00
jlxq0 deleted branch feat/stalwart-app-password 2026-08-29 12:10:15 +00:00
Author
Owner

Correcting my comment above: ci.yml pins 1.93.0 and I read a comment as the toolchain.

ci.yml:33   - uses: dtolnay/rust-toolchain@4360b525…  # stable
ci.yml:35       toolchain: 1.93.0

The # stable is a comment on the action's sha. The toolchain is two lines below it and I stopped at the line that looked like an answer, which is this fleet's own rule about reading the field name and then what follows it, arriving in my own hands.

Verified on the pinned toolchain here:

cargo +1.93.0 clippy --all-targets --all-features --locked -- -D warnings   rc=0, clean

So there was never a disagreement between local and CI. There was a disagreement between two compilers, one of which gates nothing.

And the fix I proposed would have broken the build

#[allow(clippy::unused_async_trait_impl)] is an unknown lint to 1.93.0, and -D unknown-lints is implied by -D warnings, so adding it turns a green CI red. AGENTS.md already carries that rule and it would need #[allow(unknown_lints)] above it to be safe, which today buys nothing because the compiler that emits the lint is not the one that gates.

withings-mcp's answer does not transfer and I said it did. That repository pins 1.98.0 for exactly this lint and documents the two-floor reasoning; here the floor is 1.93.0 and the lint does not exist.

What my conclusion should have been

A local gate cannot contradict CI here, not because CI moves but because my local was not the pinned toolchain. RUSTUP_TOOLCHAIN unset and a shim resolving stable makes a local result read as authoritative on the wrong compiler, which is in AGENTS.md and has now caught two of us.

cargo +1.93.0 clippy is the command that means something, and it is what I should have run before writing anything about CI.

The merge stands. Nothing was waved through: the only red came from a compiler nothing runs.

**Correcting my comment above: `ci.yml` pins `1.93.0` and I read a comment as the toolchain.** ci.yml:33 - uses: dtolnay/rust-toolchain@4360b525… # stable ci.yml:35 toolchain: 1.93.0 **The `# stable` is a comment on the action's sha.** The toolchain is two lines below it and I stopped at the line that looked like an answer, which is this fleet's own rule about reading the field name and then what follows it, arriving in my own hands. **Verified on the pinned toolchain here:** cargo +1.93.0 clippy --all-targets --all-features --locked -- -D warnings rc=0, clean **So there was never a disagreement between local and CI. There was a disagreement between two compilers**, one of which gates nothing. ## And the fix I proposed would have broken the build **`#[allow(clippy::unused_async_trait_impl)]` is an unknown lint to 1.93.0**, and `-D unknown-lints` is implied by `-D warnings`, so adding it turns a green CI red. **`AGENTS.md` already carries that rule** and it would need `#[allow(unknown_lints)]` above it to be safe, which today buys nothing because the compiler that emits the lint is not the one that gates. **`withings-mcp`'s answer does not transfer** and I said it did. That repository pins `1.98.0` for exactly this lint and documents the two-floor reasoning; here the floor is 1.93.0 and the lint does not exist. ## What my conclusion should have been **A local gate cannot contradict CI here, not because CI moves but because my local was not the pinned toolchain.** `RUSTUP_TOOLCHAIN` unset and a shim resolving `stable` makes a local result read as authoritative on the wrong compiler, which is in `AGENTS.md` and has now caught two of us. **`cargo +1.93.0 clippy` is the command that means something**, and it is what I should have run before writing anything about CI. **The merge stands.** Nothing was waved through: the only red came from a compiler nothing runs.
jlxq0 referenced this pull request from a commit 2026-08-29 12:14:04 +00:00
Sign in to join this conversation.
No reviewers
No labels
waiting-on-julian
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
jlxq0/jmap-mcp!25
No description provided.