fix(server): conflict on an ssh key the caller already enrolled

[otavio]

What

Enrolling an SSH public key that the caller already holds now answers 409 Conflict instead of 200 with the pre-existing record.

Why

enrollSSHIdentity treated same-fingerprint-same-principal as an idempotent success, returning the existing *models.SSHIdentity with a nil error. The handler then responded 200 with the old record — indistinguishable from a real creation, so the frontend had nothing to key off and the person saw no feedback at all. This aligns the endpoint with CreatePublicKey, which already returns ErrPublicKeyDuplicated for a taken fingerprint.

Closes #6846

Changes

Server

• enrollSSHIdentity: strict. Any fingerprint already taken in the namespace — by this principal or another — returns NewErrSSHIdentityDuplicated, which the existing ErrCodeDuplicated -> http.StatusConflict mapping in converter.FromErrServiceToHTTPStatus turns into a 409.
• reenrollSSHIdentity: new, and the reason the strict change is safe. An SSH approval can legitimately be confirmed twice for the same key, so applySSHApproval enrolls through this variant, which still returns the existing binding for the same principal. A fingerprint held by a different principal conflicts on both paths.
• resolveEnrolledSSHIdentity / persistSSHIdentity: the shared halves both paths need. Each enroll path performs exactly one store lookup — the split adds no query.

UI

The identity drawer already branched on 409 to show "This key is already an identity in this namespace.", so the reported symptom needed no frontend change. But ConnectDrawer's browser-key consent flow did: it asks the server whether the key is registered before enrolling, and a second tab enrolling the same key in between previously fell into the idempotent 200. Under the strict server it would have hit 409 and refused the connection the person had just consented to.

• isAlreadyEnrolled: one named predicate for the status, replacing the drawer's inline isSdkError(err) && err.status === 409.
• enrollAndConnect: swallows that specific conflict and connects. The key is one only this browser holds privately, so a 409 on it can only be an enrollment of the caller's own.

No API-surface change was needed — openapi/spec/paths/api@ssh-identities.yaml already documented 409.

Testing

Reviewers should focus on the enroll/reenroll split rather than the error mapping.

• The behaviour that separates the two variants is the reenroll same-principal case returning the existing row — TestReenrollSSHIdentity asserts the returned ID, not just a nil error, so it cannot pass on nil, nil.
• Service-account creation is unaffected by design, not by accident: it enrolls against a user ID minted inside the same transaction, so the same-principal branch was already unreachable there.
• The ConnectDrawer change itself is untested. That component has no test harness and standing one up needs WebCrypto Ed25519 plus IndexedDB fakes; the extracted predicate is covered instead. Worth exercising by hand: open the connect drawer in two tabs on a browser with no enrolled key and confirm consent in both — the second should connect rather than show "Could not register this browser."

Server suite (27 packages) and golangci-lint run ./... clean; console build, lint, and 3000 tests clean.

[github-actions]

Claude finished @otavio's task in 1m 9s —— View job

---

Code Review Complete

Reviewed 3 files across code quality, security, testing, language patterns, and architecture — no new issues found. The code looks good as-is.

A few things I verified while reviewing:

• All three callers behave as documented. CreateSSHIdentity now correctly gets the strict path (409 on same-caller duplicate, covered by the new test). applySSHApproval switched to reenrollSSHIdentity for replay tolerance. service-account.go:87 still uses enrollSSHIdentity and is genuinely unaffected — the PrincipalID is a fresh user ID minted inside the same transaction, so the same-principal branch is unreachable and a cross-principal fingerprint clash conflicted on both the old and new paths.
• The resolve/persist split adds no extra query — each enroll path performs exactly one SSHIdentityResolve, matching the prior behavior.
• The insert race is still handled — persistSSHIdentity maps store.ErrDuplicate to NewErrSSHIdentityDuplicated, so a concurrent create between resolve and insert still surfaces a 409 rather than a raw store error.
• Error → HTTP mapping is intact — NewErrSSHIdentityDuplicated carries ErrCodeDuplicated, which converter.FromErrServiceToHTTPStatus maps to 409, consistent with ErrPublicKeyDuplicated.
• Tests assert the meaningful distinction — TestReenrollSSHIdentity checks the returned ID on the same-principal case, so it cannot pass on a nil, nil short-circuit.

The ConnectDrawer.tsx race gap you flagged in the PR description is a fair, clearly-scoped follow-up rather than a blocker for this change.

---

To request another review round, comment /review.

[otavio]

/review

[github-actions]

Claude finished @otavio's task in 3m 18s —— View job

---

Code Review Complete

Reviewed 7 files across code quality, security, testing, language patterns, and architecture — no new issues found. The code looks good as-is.

Highlights verified this round:

• Enroll/reenroll split is correct. enrollSSHIdentity conflicts on any existing holder; reenrollSSHIdentity returns the existing binding only when existing.PrincipalID == identity.PrincipalID and still 409s for a different principal. resolveEnrolledSSHIdentity/persistSSHIdentity are cleanly factored with no extra query per path.
• service-account.go:87 is genuinely unaffected. The PrincipalID is a user ID minted inside the same WithTransaction, so the same-principal branch is unreachable and both reachable outcomes return exactly as before.
• No cross-repo breakage. The changed methods are unexported and absent from the exported coreservices.Service interface; a grep of cloud/ finds only the autogenerated store mock. No override, wrap, or call.
• Security is sound. The swallowed 409 in ConnectDrawer.enrollAndConnect matches strictly status === 409 on a browser-held non-extractable Ed25519 key, so a cross-principal fingerprint collision is cryptographically infeasible — no connect-with-a-key-you-don't-hold path. Tenant binding via BoundTo is intact.
• Tests assert the meaningful distinction. TestReenrollSSHIdentity pins the returned ID (can't pass on nil, nil), the flipped TestEnrollSSHIdentity case expects NewErrSSHIdentityDuplicated, and isAlreadyEnrolled covers 409/other-status/non-SDK inputs. Mock .Once() + AssertExpectations match one resolve + one create per path.
• Error → HTTP mapping confirmed: NewErrSSHIdentityDuplicated carries ErrCodeDuplicated → http.StatusConflict.

---

To request another review round, comment /review.