sneak/webhooker
1
0
Fork 0
Code Issues 49 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

Add a webhooker resetpw subcommand and a bootstrap banner (closes #208) #239

Merged
clawbot merged 1 commits from issue-208-admin-password-recovery into next 2026-08-20 08:01:42 +02:00
Conversation 1 Commits 1 Files Changed 10 +1431 -18
clawbot commented 2026-08-20 07:49:40 +02:00
Collaborator
Copy Link

Closes #208.

The bootstrap admin password was shown exactly once, as one INFO
record among the ~45 fx lines a boot writes, and there was no reset
path at all. Losing that line meant deleting the users row from
webhooker.db by hand.

What landed

  • internal/banner renders the one credential shown in the clear
    as a ruled block written straight to standard output, so it does not
    read as one more log line. The first boot emits the password there
    and nowhere else — the old d.log.Info("admin user created", "password", ...) field is gone, so the plaintext still appears
    exactly once — and the banner names the recovery command.
  • webhooker resetpw [-generate] <username> sets an existing
    account's password. Hashing goes through database.HashPassword, so
    the Argon2id parameters cannot drift into a second implementation.
  • It refuses against a live instance: it takes the same exclusive
    flock from internal/datadir the server takes, detects the
    contended case with errors.Is(err, datadir.ErrLocked), and releases
    it on the way out.
  • database.Open exposes the connect-and-migrate path without fx
    and without seeding; seeding moved to ensureAdminUser, which only a
    server start calls. A maintenance command must not answer "no such
    user" by creating one.
  • main gains subcommand dispatch. No arguments still runs the
    server on the unchanged path: the DATA_DIR lock is taken inside
    run before the fx graph is built, and fx still owns the non-zero
    exit (#218). An unknown
    subcommand exits 2 rather than starting a server, so resetpww does
    not silently take the lock and serve. Stdlib flag; no new
    dependency.
  • README documents the bootstrap banner and the recovery command,
    including the container invocation (the image sets CMD, not
    ENTRYPOINT, so the whole command has to be given) and the three
    things resetpw will not do.

Decisions the issue left open

The MkdirAll side effect: not reached. datadir.Acquire creates
the directory, so resetpw stats DATA_DIR and {DATA_DIR}/ webhooker.db before taking the lock and errors if either is
missing. A mistyped path is never built out, no lock file is left in a
wrong directory, and no empty schema is migrated into one — otherwise
the command would report success against a deployment that does not
exist while the real one stayed locked out. Two tests pin it: a missing
DATA_DIR must still not exist afterwards, and a directory with no
database must still be empty (lock file included).

Password input: stdin or -generate, never argv. On Linux argv is
readable through /proc by every account on the host for as long as
the process lives, and it lands in shell history. -generate uses the
existing GenerateRandomPassword, which is crypto/rand throughout
(rand.Int over crypto/rand.Reader). Standard input is read as one
line, minimum 8 bytes — a floor against a stray keystroke or a
truncated pipe becoming the account's only credential, not a policy.
When stdin is a terminal it prompts and says the input is echoed; no
terminal library is pulled in to turn echo off.

Unknown user: hard error. resetpw changes an existing account's
password and never creates one, so a typo says so instead of quietly
adding a second user. Tested, including that the user count stays 1.

No half-written state. The account is resolved and the hash
computed in full before the single UPDATE that stores it, so any
failure leaves the stored credential untouched — asserted by the
refusal, unknown-user and rejected-password tests, which all re-read
the stored hash. A supplied password is not echoed back; a generated
one falls back to stderr if stdout cannot be written, because at that
point it is already stored and losing it locks the deployment out.

Test cost. One fx application in the whole new package, shared by
the reset-then-login test, and the deployment fixture inserts its user
row directly rather than booting the graph to spend an Argon2id hash on
a password no test can use. internal/handlers gained nothing.

Gate

Load average during the runs: 44-120 across 48 cores (uptime
before/after each: 82.67 → 74.58 for the tests, 44.34 after lint).

make test (rebased onto aba02bc, GOFLAGS=-count=1, zero
(cached) lines):

ok  	sneak.berlin/go/webhooker/cmd/webhooker	1.081s
ok  	sneak.berlin/go/webhooker/internal/banner	1.024s
ok  	sneak.berlin/go/webhooker/internal/database	3.100s
ok  	sneak.berlin/go/webhooker/internal/resetpw	4.014s
ok  	sneak.berlin/go/webhooker/internal/handlers	27.976s
FAIL	sneak.berlin/go/webhooker/internal/gormlog	1.053s

The new tests, all passing:

--- PASS: TestResetThenLogin (0.80s)
--- PASS: TestGeneratedPasswordIsPrintedAndWorks (0.63s)
--- PASS: TestGeneratedPasswordSurvivesAFailedStdout
--- PASS: TestRefusesLiveInstance (0.21s)
--- PASS: TestMissingDataDirCreatesNothing (0.00s)
--- PASS: TestMissingDatabaseCreatesNothing (0.00s)
--- PASS: TestUnknownUserFails (0.37s)
--- PASS: TestRejectsUnusablePasswords (0.50s)
--- PASS: TestUsageErrors (0.00s)
--- PASS: TestFirstBoot_PrintsTheAdminPasswordAsABanner (1.33s)
--- PASS: TestDispatch_NoArgumentsRunsTheServer (0.00s)
--- PASS: TestDispatch_UnknownSubcommand (0.00s)
--- PASS: TestDispatch_Help (0.00s)
--- PASS: TestCredentials_IsFindableByEye (0.00s)
--- PASS: TestCredentials_ReportsAWriteFailure (0.00s)

make check is red, and not on this branch's account. The single
failure is TestGormScanIsNeverCalledOutsideTests, which fails on
next itself — internal/delivery/queue_depth.go:109 and :161 call
(*gorm.DB).Scan. Already filed as
#234; nothing here touches
that file. Because script/check stops at the first failing step, lint
and fmt-check were run as their own targets:

make lint      -> #11 [lint 3/3] RUN golangci-lint run --config .golangci.yml ./...
                  #11 59.82 0 issues.
                  #11 DONE 60.9s
make fmt-check -> exit 0

docker build --no-cache-filter=lint --no-cache-filter=builder --progress=plain . (run before the rebase, on the same tree):

#17 [lint 9/9] RUN --network=none golangci-lint run --config .golangci.yml ./...
#17 61.13 0 issues.
#17 DONE 63.4s
#30 [builder 8/9] RUN make test
#30 70.55 ok  	sneak.berlin/go/webhooker/internal/banner	1.085s
#30 76.63 ok  	sneak.berlin/go/webhooker/internal/database	5.092s
#30 121.5 ok  	sneak.berlin/go/webhooker/internal/resetpw	6.995s
#30 121.5 ok  	sneak.berlin/go/webhooker/internal/handlers	51.275s
#30 77.16 FAIL	sneak.berlin/go/webhooker/internal/gormlog	1.593s
#30 ERROR: process "/bin/sh -c make test" did not complete successfully: exit code: 2

Real durations on both stages, no CACHED on either, every other
package green inside the container — internal/handlers at 51.3s under
load 120, well inside the 90s budget. The build cannot reach the
runtime stage until issue 234 is fixed, which is not this unit's
change. No image was produced; nothing of mine is left in docker ps -a or docker image ls. No prune was run.

Closes https://git.eeqj.de/sneak/webhooker/issues/208. The bootstrap admin password was shown exactly once, as one `INFO` record among the ~45 fx lines a boot writes, and there was no reset path at all. Losing that line meant deleting the `users` row from `webhooker.db` by hand. ## What landed - **`internal/banner`** renders the one credential shown in the clear as a ruled block written straight to standard output, so it does not read as one more log line. The first boot emits the password there and **nowhere else** — the old `d.log.Info("admin user created", "password", ...)` field is gone, so the plaintext still appears exactly once — and the banner names the recovery command. - **`webhooker resetpw [-generate] <username>`** sets an existing account's password. Hashing goes through `database.HashPassword`, so the Argon2id parameters cannot drift into a second implementation. - It **refuses against a live instance**: it takes the same exclusive `flock` from `internal/datadir` the server takes, detects the contended case with `errors.Is(err, datadir.ErrLocked)`, and releases it on the way out. - **`database.Open`** exposes the connect-and-migrate path without fx and without seeding; seeding moved to `ensureAdminUser`, which only a server start calls. A maintenance command must not answer "no such user" by creating one. - **`main` gains subcommand dispatch.** No arguments still runs the server on the unchanged path: the `DATA_DIR` lock is taken inside `run` before the fx graph is built, and fx still owns the non-zero exit (https://git.eeqj.de/sneak/webhooker/pulls/218). An unknown subcommand exits 2 rather than starting a server, so `resetpww` does not silently take the lock and serve. Stdlib `flag`; no new dependency. - README documents the bootstrap banner and the recovery command, including the container invocation (the image sets `CMD`, not `ENTRYPOINT`, so the whole command has to be given) and the three things `resetpw` will not do. ## Decisions the issue left open **The `MkdirAll` side effect: not reached.** `datadir.Acquire` creates the directory, so `resetpw` stats `DATA_DIR` and `{DATA_DIR}/ webhooker.db` **before** taking the lock and errors if either is missing. A mistyped path is never built out, no lock file is left in a wrong directory, and no empty schema is migrated into one — otherwise the command would report success against a deployment that does not exist while the real one stayed locked out. Two tests pin it: a missing `DATA_DIR` must still not exist afterwards, and a directory with no database must still be empty (lock file included). **Password input: stdin or `-generate`, never argv.** On Linux argv is readable through `/proc` by every account on the host for as long as the process lives, and it lands in shell history. `-generate` uses the existing `GenerateRandomPassword`, which is `crypto/rand` throughout (`rand.Int` over `crypto/rand.Reader`). Standard input is read as one line, minimum 8 bytes — a floor against a stray keystroke or a truncated pipe becoming the account's only credential, not a policy. When stdin is a terminal it prompts and says the input is echoed; no terminal library is pulled in to turn echo off. **Unknown user: hard error.** `resetpw` changes an existing account's password and never creates one, so a typo says so instead of quietly adding a second user. Tested, including that the user count stays 1. **No half-written state.** The account is resolved and the hash computed in full before the single `UPDATE` that stores it, so any failure leaves the stored credential untouched — asserted by the refusal, unknown-user and rejected-password tests, which all re-read the stored hash. A supplied password is not echoed back; a generated one falls back to stderr if stdout cannot be written, because at that point it is already stored and losing it locks the deployment out. **Test cost.** One fx application in the whole new package, shared by the reset-then-login test, and the deployment fixture inserts its user row directly rather than booting the graph to spend an Argon2id hash on a password no test can use. `internal/handlers` gained nothing. ## Gate Load average during the runs: **44-120 across 48 cores** (`uptime` before/after each: 82.67 → 74.58 for the tests, 44.34 after lint). `make test` (rebased onto `aba02bc`, `GOFLAGS=-count=1`, zero `(cached)` lines): ``` ok sneak.berlin/go/webhooker/cmd/webhooker 1.081s ok sneak.berlin/go/webhooker/internal/banner 1.024s ok sneak.berlin/go/webhooker/internal/database 3.100s ok sneak.berlin/go/webhooker/internal/resetpw 4.014s ok sneak.berlin/go/webhooker/internal/handlers 27.976s FAIL sneak.berlin/go/webhooker/internal/gormlog 1.053s ``` The new tests, all passing: ``` --- PASS: TestResetThenLogin (0.80s) --- PASS: TestGeneratedPasswordIsPrintedAndWorks (0.63s) --- PASS: TestGeneratedPasswordSurvivesAFailedStdout --- PASS: TestRefusesLiveInstance (0.21s) --- PASS: TestMissingDataDirCreatesNothing (0.00s) --- PASS: TestMissingDatabaseCreatesNothing (0.00s) --- PASS: TestUnknownUserFails (0.37s) --- PASS: TestRejectsUnusablePasswords (0.50s) --- PASS: TestUsageErrors (0.00s) --- PASS: TestFirstBoot_PrintsTheAdminPasswordAsABanner (1.33s) --- PASS: TestDispatch_NoArgumentsRunsTheServer (0.00s) --- PASS: TestDispatch_UnknownSubcommand (0.00s) --- PASS: TestDispatch_Help (0.00s) --- PASS: TestCredentials_IsFindableByEye (0.00s) --- PASS: TestCredentials_ReportsAWriteFailure (0.00s) ``` **`make check` is red, and not on this branch's account.** The single failure is `TestGormScanIsNeverCalledOutsideTests`, which fails on `next` itself — `internal/delivery/queue_depth.go:109` and `:161` call `(*gorm.DB).Scan`. Already filed as https://git.eeqj.de/sneak/webhooker/issues/234; nothing here touches that file. Because `script/check` stops at the first failing step, lint and fmt-check were run as their own targets: ``` make lint -> #11 [lint 3/3] RUN golangci-lint run --config .golangci.yml ./... #11 59.82 0 issues. #11 DONE 60.9s make fmt-check -> exit 0 ``` `docker build --no-cache-filter=lint --no-cache-filter=builder --progress=plain .` (run before the rebase, on the same tree): ``` #17 [lint 9/9] RUN --network=none golangci-lint run --config .golangci.yml ./... #17 61.13 0 issues. #17 DONE 63.4s #30 [builder 8/9] RUN make test #30 70.55 ok sneak.berlin/go/webhooker/internal/banner 1.085s #30 76.63 ok sneak.berlin/go/webhooker/internal/database 5.092s #30 121.5 ok sneak.berlin/go/webhooker/internal/resetpw 6.995s #30 121.5 ok sneak.berlin/go/webhooker/internal/handlers 51.275s #30 77.16 FAIL sneak.berlin/go/webhooker/internal/gormlog 1.593s #30 ERROR: process "/bin/sh -c make test" did not complete successfully: exit code: 2 ``` Real durations on both stages, no `CACHED` on either, every other package green inside the container — `internal/handlers` at 51.3s under load 120, well inside the 90s budget. The build cannot reach the runtime stage until issue 234 is fixed, which is not this unit's change. No image was produced; nothing of mine is left in `docker ps -a` or `docker image ls`. No prune was run.
clawbot added 1 commit 2026-08-20 07:49:40 +02:00
Add a webhooker resetpw subcommand and a bootstrap banner (closes #208)
Some checks failed
check / check (push) Failing after 2m32s
Details
29ce4cc429 
The bootstrap admin password was shown exactly once, as one INFO
record among the roughly 45 fx lines a boot writes, and there was no
reset path at all: no subcommand, no forgot-password flow, no
override. Losing that line meant deleting the users row from
webhooker.db by hand so the next start would re-seed.

- internal/banner renders the one credential shown in the clear as a
  ruled block written straight to standard output, so it does not read
  as one more log line. The first boot emits the password there and
  nowhere else, and the banner names the recovery command.
- `webhooker resetpw [-generate] <username>` sets an existing
  account's password. It reads the password as one line from standard
  input, or generates one with crypto/rand via the existing
  GenerateRandomPassword; it is never an argv value, which /proc would
  publish to every account on the host. Hashing goes through
  database.HashPassword, so the Argon2id parameters cannot drift.
- It refuses to run against a DATA_DIR a live instance holds, by
  taking the same exclusive flock internal/datadir gives the server,
  and releases it when it finishes.
- It creates nothing. A missing DATA_DIR, a directory with no
  webhooker.db, and an unknown username are each an error: datadir
  .Acquire calls os.MkdirAll, so a mistyped path would otherwise be
  built out and reported as a success. The existence checks therefore
  run before the lock is taken.
- The account is resolved and the hash computed in full before the
  single UPDATE that stores it, so any failure leaves the stored
  credential untouched.
- database.Open exposes the connect-and-migrate path without fx and
  without seeding; seeding moves to ensureAdminUser, which only a
  server start calls.
- main gains subcommand dispatch. No arguments still runs the server
  on the same path, with the DATA_DIR lock taken before the fx graph
  is built and fx owning the non-zero exit; an unknown subcommand
  exits 2 rather than starting a server.

Tests: reset then log in through the real form POST handler, the
generated password verifying against the stored hash, the refusal
against a held lock, both create-nothing cases, the unknown user, the
unusable passwords, and the first-boot banner carrying a password that
opens the account.

README documents the bootstrap banner and the recovery command,
including the container invocation and what resetpw will not do.
clawbot added the needs-review label 2026-08-20 07:49:44 +02:00
clawbot self-assigned this 2026-08-20 07:49:44 +02:00
clawbot commented 2026-08-20 07:56:18 +02:00
Author
Collaborator
Copy Link

PASS — independent review of #239 found no blocking defects.

Disclosure: the authoritative gate re-run on 29ce4cc (docker build --no-cache-filter=lint --no-cache-filter=builder) is red, solely at #32 [builder 9/11] RUN make test on TestGormScanIsNeverCalledOutsideTests, naming only internal/delivery/queue_depth.go:109:3 and :161:3 — #234, not attributable here. Lint ran uncached (#19 [lint 9/9], 61.4s, 0 issues.), #17 make fmt-check clean, zero (cached) go-test markers, every other package ok. Merges cleanly into next (verified by local test-merge, not Gitea's flag).

PASS — independent review of https://git.eeqj.de/sneak/webhooker/pulls/239 found no blocking defects. Disclosure: the authoritative gate re-run on `29ce4cc` (`docker build --no-cache-filter=lint --no-cache-filter=builder`) is red, solely at `#32 [builder 9/11] RUN make test` on `TestGormScanIsNeverCalledOutsideTests`, naming only `internal/delivery/queue_depth.go:109:3` and `:161:3` — https://git.eeqj.de/sneak/webhooker/issues/234, not attributable here. Lint ran uncached (`#19 [lint 9/9]`, 61.4s, `0 issues.`), `#17 make fmt-check` clean, zero `(cached)` go-test markers, every other package `ok`. Merges cleanly into `next` (verified by local test-merge, not Gitea's flag).
clawbot merged commit 9969694a47 into next 2026-08-20 08:01:42 +02:00
clawbot deleted branch issue-208-admin-password-recovery 2026-08-20 08:01:42 +02:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
merge-ready
needs-checks
needs-rebase
needs-review
needs-rework
No Label needs-review
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
clawbot sneak
No Assignees clawbot
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/webhooker#239
Delete Branch "issue-208-admin-password-recovery"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?