Skip to content

fix(desktop): normalize agent pubkeys through one helper, and guard it - #6235

Open
mfethe1 wants to merge 1 commit into
block:mainfrom
mfethe1:fix/agent-pubkey-normalization
Open

fix(desktop): normalize agent pubkeys through one helper, and guard it#6235
mfethe1 wants to merge 1 commit into
block:mainfrom
mfethe1:fix/agent-pubkey-normalization

Conversation

@mfethe1

@mfethe1 mfethe1 commented Aug 18, 2026

Copy link
Copy Markdown

fix(desktop): normalize agent pubkeys through one helper, and guard it

normalizePubkey is trim().toLowerCase(). Across features/agents, 20 call
sites reached for a bare .toLowerCase() instead. The two agree for every
well-formed pubkey and disagree the moment one carries surrounding whitespace,
so the divergence is invisible until a value arrives from somewhere untidy — a
pasted allowlist entry, a relay tag, a config file — and then one surface stops
matching another with nothing thrown and nothing logged.

That is the same failure shape the agent-identity work keeps turning up: two
places answer "is this the same agent?" differently, and a user reports that an
agent "isn't there".

One site was already asymmetric rather than merely inconsistent.
RespondToField.handleRemove compared p.toLowerCase() against a pubkey that
had been normalized, so removing an allowlist entry could fail to match the
entry it was given. That is a live bug, not a style point.

Guard

check-pubkey-normalization.mjs matches any .toLowerCase() call and then
filters on the receiver, rather than anchoring the pubkey inside the pattern.
The first version did anchor it, and could not see
agent?.pubkey.toLowerCase() — the optional link sits between the identifier
and the segment being matched. Filtering on the receiver found three
optional-chained sites the initial sweep had skipped, two of which the anchored
pattern could never have reported.

Scoped to src/features/agents deliberately. Hand-rolled lowercasing is
repo-wide — 87 files under desktop/src against 113 using normalizePubkey
and failing on all of them would make the guard unshippable. A guard that must
be disabled to land anything protects nothing. So it covers the surface whose
divergence caused an outage, and covers it completely: no allowlist. Widening it
is a follow-up that has to arrive with the call-site fixes, not a flag flip.

Interaction with #6077

Both this PR and #6077 add a check:* script and extend the same check line
in desktop/package.json, so whichever lands second needs a one-line rebase
there. Nothing else overlaps: #6077 guards how an identity KEY is minted, this
guards how the pubkey inside it is normalized.

Note that main recently removed pnpm check:file-sizes from that line (#6187
made the ratchet a first-class root gate). Neither PR should put it back.

Verification

npx tsc --noEmit clean. pnpm check clean. Desktop unit suite: 4993 passed,
0 failed.

Guard proven to fail, not just to pass: an injected x.pubkey.toLowerCase()
under features/agents is reported and exits 1; removing it exits 0.

Signed-off-by: Michael Feth michael@jira-flow.com

`normalizePubkey` is `trim().toLowerCase()`. Across `features/agents`, 20 call
sites reached for a bare `.toLowerCase()` instead. The two agree for every
well-formed pubkey and disagree the moment one carries surrounding whitespace,
so the divergence is invisible until a value arrives from somewhere untidy — a
pasted allowlist entry, a relay tag, a config file — and then one surface stops
matching another with nothing thrown and nothing logged.

That is the same failure shape the agent-identity work keeps turning up: two
places answer "is this the same agent?" differently, and a user reports that an
agent "isn't there".

One site was already asymmetric rather than merely inconsistent.
`RespondToField.handleRemove` compared `p.toLowerCase()` against a pubkey that
had been normalized, so removing an allowlist entry could fail to match the
entry it was given. That is a live bug, not a style point.

## Guard

`check-pubkey-normalization.mjs` matches any `.toLowerCase()` call and then
filters on the receiver, rather than anchoring the pubkey inside the pattern.
The first version did anchor it, and could not see
`agent?.pubkey.toLowerCase()` — the optional link sits between the identifier
and the segment being matched. Filtering on the receiver found three
optional-chained sites the initial sweep had skipped, two of which the anchored
pattern could never have reported.

Scoped to `src/features/agents` deliberately. Hand-rolled lowercasing is
repo-wide — 87 files under `desktop/src` against 113 using `normalizePubkey` —
and failing on all of them would make the guard unshippable. A guard that must
be disabled to land anything protects nothing. So it covers the surface whose
divergence caused an outage, and covers it completely: no allowlist. Widening it
is a follow-up that has to arrive with the call-site fixes, not a flag flip.

## Interaction with block#6077

Both this PR and block#6077 add a `check:*` script and extend the same `check` line
in `desktop/package.json`, so whichever lands second needs a one-line rebase
there. Nothing else overlaps: block#6077 guards how an identity KEY is minted, this
guards how the pubkey inside it is normalized.

Note that `main` recently removed `pnpm check:file-sizes` from that line (block#6187
made the ratchet a first-class root gate). Neither PR should put it back.

## Verification

`npx tsc --noEmit` clean. `pnpm check` clean. Desktop unit suite: 4993 passed,
0 failed.

Guard proven to fail, not just to pass: an injected `x.pubkey.toLowerCase()`
under `features/agents` is reported and exits 1; removing it exits 0.

Signed-off-by: Michael Feth <michael@jira-flow.com>
@mfethe1
mfethe1 requested a review from a team as a code owner August 18, 2026 16:04
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.

1 participant