fix(agentgit): register the member's SSH key on every BBS login, not just at signup - #131
Merged
Merged
Conversation
…just at signup
provisionGit returned early when the Forgejo account already existed, so
EnsureKey only ever ran during first provisioning. A member who deleted their
key on git.profullstack.com never got it back: every later BBS login hit the
`if !created { return }` and skipped key registration entirely. The comment on
the web verify path ("key is added on next BBS login") was describing behaviour
that could not happen.
Key registration now runs on every provisionGit call that carries a session
key. EnsureKey was already idempotent — it GETs the account's keys and compares
key material ignoring the comment — so re-running it is free when nothing
changed, and it re-adds a removed key, picks up a rotated one, and backfills
members who joined before AgentGit captured keys. The welcome email stays gated
on `created`, since the one-time password is only meaningful for a new account.
Two things that would have made this unreliable in the new every-login path:
- The key title was the constant "agentbbs". Forgejo rejects a duplicate title
with 422, so a member who rotated their BBS key would have had the new one
silently dropped. The title now carries a short fingerprint, so distinct keys
coexist and the same key stays stable across logins.
- EnsureKey mapped *every* 422 to "already exists" and returned nil. That hid
genuine rejections forever, which matters far more now the call is on the hot
path. Only "has been used" bodies are swallowed; a rejected key surfaces with
Forgejo's own reason so it lands in the logs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ThreatCrush Security Scan10 finding(s) HIGH/CRITICAL: 7 | LOW: 3
Snippets are redacted; ThreatCrush never prints matched credential material. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
When a member joins the BBS, their SSH key is supposed to be registered on their
git.profullstack.comaccount — "your BBS SSH key is your git key", as the podbanner says. It only ever happened once, at first provisioning.
provisionGitbailed out before reachingEnsureKey:So a member who deleted their key on AgentGit never got it back, and members who
joined before AgentGit captured keys were never backfilled — even though the
login path at
main.go:519callsprovisionGiton every visit specifically todo that, and its comment says so. The web verify path's comment ("key is added
on next BBS login",
main.go:1217) described behaviour that could not happen.Change
Key registration now runs on every
provisionGitcall that carries a sessionkey.
EnsureKeywas already idempotent — it GETs the account's keys andcompares key material ignoring the trailing comment — so re-running it costs one
GET when nothing changed, and it re-adds a removed key, picks up a rotated one,
and backfills old accounts. The welcome email stays gated on
created, sincethe one-time password is empty for an account that already existed.
Two things that would have made the every-login path unreliable:
"agentbbs". Forgejo 422s a duplicate title,so a member who rotated their BBS key would have had the new key silently
dropped. The title now carries a short fingerprint (
agentbbs <fp>), sodistinct keys coexist and the same key stays stable across logins.
EnsureKeymapped every 422 to "already exists" and returnednil. Thathid genuine rejections forever, which matters much more now the call is on the
hot path. Only "has been used" bodies are swallowed; anything else surfaces
with Forgejo's own reason so it reaches the logs.
Tests
TestProvisionGitRegistersKeyForExistingAccount— the regression guard: withan account that already exists, the key must still be POSTed. Fails against
the old early-return.
TestGitKeyTitleIsPerKey— distinct keys get distinct titles, the same key isstable, unparseable input degrades to
"agentbbs".TestEnsureKeyTreatsAlreadyUsed422AsBenign/TestEnsureKeySurfacesRejecting422— the two sides of the 422 split.
gofmt,go vetand the fullgo test ./...suite are clean.Not covered here
Pushing over SSH to AgentGit is still broken for an unrelated reason: Forgejo
advertises
ssh://git@git.profullstack.com:2222/…, but 2222 is not reachable —it times out, while 587 on the same host answers refused, so the live ufw
ruleset has no ACCEPT for it despite
setup.shcarryingufw allow "${FORGEJO_SSH_PORT}/tcp"since f210b42. Port 22 there is agentbbsitself, so the pod banner's
git push → git@git.profullstack.comfails withfatal: protocol error: bad line length character: Requ(the BBS's "Requires anactive PTY" parsed as git protocol). Needs a look on the box; HTTPS remotes work
today.
🤖 Generated with Claude Code