Skip to content

security: remediate 2026-07-16 audit findings - #44

Merged
MalteKiefer merged 10 commits into
mainfrom
security/audit-2026-07-16-remediation
Jul 16, 2026
Merged

MalteKiefer merged 10 commits into
mainfrom
security/audit-2026-07-16-remediation

Conversation

@MalteKiefer

Copy link
Copy Markdown
Owner

Remediates the source-fixable findings from the 2026-07-16 security audit (see SECURITY_AUDIT_2026-07-16.md). All commits are SSH-signed; go build/go vet/staticcheck clean, govulncheck reports 0, tests pass.

Fixed

Critical / High

  • C1 (code) — agent self-update is now fail-closed: removed the PublicKey=="" skip branch; a failed minisign verification is never installed, even under MON_AGENT_UPDATE_INSECURE (which may only skip fetching a signature).
  • H1 — probe SSRF: loopback + link-local (incl. 169.254.169.254 metadata) always denied; RFC1918/ULA denied by default, opt-in MON_PROBE_ALLOW_INTERNAL=1.
  • H2 — Go 1.26.5, x/image/x/crypto/x/net bumped, CI go-version synced, Vite 6.4.3. govulncheck 0.
  • H3 — webhook SSRF: dial-time net.Dialer.Control guard (beats DNS rebinding + redirects), http/https scheme allowlist, 10-hop redirect cap.

Medium

  • M2 — TOTP seeds encrypted at rest (AES-256-GCM, opt-in MON_DATA_ENCRYPTION_KEY, dual-read); backup codes bcrypt-hashed. No forced migration.
  • M3 — compliance middleware fails closed on lookup error.
  • M4 — trustedRealIP: forwarded headers honoured only from trusted peers (MON_TRUSTED_PROXIES).
  • M5 — release container/publish jobs gated behind a protected release environment.
  • M6 — prod compose pins the image by immutable @${MONSYS_DIGEST}.
  • M7 — TOTP replay protection (migration 0034, single-use step, ASVS V2.8.4).
  • M8 — TLS AEAD-only cipher suites; plain-HTTP startup requires MON_ALLOW_INSECURE_HTTP=1.
  • M9 — retention lowered to 90d (migration 0033) to match PRIVACY.md.

Low / cleanup — L1 (account enumeration), L3 (/readyz leak), L4 (WebAuthn prod hard-fail), I1/I3 + dead-code removal.

New environment variables

  • MON_DATA_ENCRYPTION_KEY — base64 32-byte AES-256 key; opt-in TOTP-seed encryption at rest.
  • MON_ALLOW_INSECURE_HTTP=1 — permit plain-HTTP startup behind a TLS proxy.
  • MON_TRUSTED_PROXIES — CIDRs whose forwarded headers are trusted (default: loopback + RFC1918/ULA).

Still owner-only (not source-fixable)

  • C1 key wiring — real minisign keypair + release ldflags + deploy/release.pub + hard-fail CI signing.
  • M5 reviewer policy — add required reviewers to the release environment in repo Settings.
  • L5 — needs a store.ValidationError type to split user-safe validation from wrapped DB errors.
  • Optional backfill migration to re-encrypt/re-hash existing TOTP rows once the key is set.

…L3/I1/I3)

Agent self-update (C1/M1): verification now always runs and is fail-closed.
Remove the PublicKey=="" fail-open skip branch; a failed minisign
verification is never installed, even under MON_AGENT_UPDATE_INSECURE
(which may now only skip *fetching* a signature).

SSRF (H1): probe target guard is two-tier and fail-closed — loopback and
link-local (incl. 169.254.169.254 metadata) are always denied; RFC1918/ULA
denied by default, opt-in via MON_PROBE_ALLOW_INTERNAL=1.

SSRF (H3): outbound webhooks get a dial-time net.Dialer.Control guard that
blocks loopback/link-local for every backend and redirect hop (defeats DNS
rebinding), an http/https scheme allowlist, and a 10-hop redirect cap.

Dependencies (H2): go 1.26.5, x/image v0.44.0, x/crypto v0.54.0,
x/net v0.57.0, CI go-version 1.26.5, Vite 6.4.3. govulncheck now clean.

Also: compliance middleware fails closed on lookup error (M3); disabled
account no longer distinguishable at login/passkey (L1); /readyz no longer
echoes the DB error (L3); removed no-op ConstantTimeCompare (I1); pre-commit
golangci-lint bumped to v2.11.4 to match CI (I3).
…nup)

M7: TOTP replay protection. Add user_totp.last_used_step (migration 0034) and
auth2fa.ValidateAndStep; VerifyTOTP rejects a step already consumed so a code
observed within its skew window cannot be replayed (ASVS V2.8.4). No separate
2FA lockout added — the challenge token is single-use, so a wrong code forces a
fresh, lockout-protected password login.

M9: migration 0033 lowers login_events / alert_history / audit_log retention to
90 days to match the published PRIVACY.md statement.

L2: release.yaml root token is now contents: read; contents: write is scoped to
the auto-tag and publish jobs only.

L4: refuse to start with the localhost WebAuthn RP fallback unless
MON_ENV=development.

Cleanup: remove dead var _ = sql.ErrNoRows / pgx.ErrNoRows and their now-unused
imports.
M2: encryption-at-rest for TOTP seeds (AES-256-GCM, internal/server/store/
secrets.go) opt-in via MON_DATA_ENCRYPTION_KEY, with dual-read so legacy
plaintext rows keep working (no forced migration). Backup codes are now
bcrypt-hashed; MatchAndConsume verifies hash-or-legacy-plaintext. Server fails
fast on a malformed key. Unit tests cover the crypto round-trip, wrong-key
failure, legacy passthrough, backup-code consume, and the replay step.

M8: TLS 1.2 floor with AEAD-only ECDHE cipher suites; the server refuses to
start on plain HTTP unless MON_ALLOW_INSECURE_HTTP=1 (explicit behind-proxy
opt-in), since it emits Secure/HSTS cookies.

M4: replace middleware.RealIP with trustedRealIP — X-Forwarded-For / X-Real-IP
are honoured only when the direct peer is loopback/RFC1918 (default) or listed
in MON_TRUSTED_PROXIES, so a directly-reachable client cannot spoof its IP.

M5: gate the container and publish release jobs behind a protected "release"
environment for change management (reviewers configured in repo settings).
… (M6)

AUDIT-2026-07-16 M6: docker-compose.prod.yaml pinned ghcr.io/...:${MONSYS_TAG}
with pull_policy: always. A tag is mutable on GHCR, so an operator could
cosign-verify a digest and then deploy a different image that was repushed
behind the same tag (TOCTOU). Pin by @${MONSYS_DIGEST} (the cosign-verified
value from container-digest.txt) and drop pull_policy: always — a digest is
content-addressed and immutable. Docs (RELEASE.md, OPERATIONS.md) updated to
match.
…(C1/L5)

C1 (release wiring): the build step now embeds updater.PublicKey from
deploy/release.pub via ldflags when the file is present (a missing key
degrades to fail-closed no-auto-update, never unsigned). The minisign signing
step is now REQUIRED — it fails the release instead of publishing unsigned
artifacts when the secrets are missing. verify.go docs reconciled to the single
deploy/release.pub path. The maintainer still supplies the actual keypair
(commit deploy/release.pub, set MONSYS_MINISIGN_SECRET_KEY/_PASSWORD secrets).

L5: route the 23 huma.Error400BadRequest(err.Error()) handler sites through a
badRequest helper that surfaces validation messages but sends wrapped Postgres
errors (pgconn.PgError) through internalErr, so raw table/column/constraint/SQL
detail never reaches the client.
The commit-signing job ran `git log %G?` without an allowed-signers file, so
SSH-signed commits could never resolve to G/U and every signed PR failed. Add
.github/allowed_signers with the maintainer signing key and point
gpg.ssh.allowedSignersFile at it in the job so signatures actually verify.
golangci gocritic importShadow (notify.go) + staticcheck QF1001 (totp_test.go).
Commit the minisign public key (deploy/release.pub) so the release build embeds
it via ldflags into mon-agent — the self-update trust anchor is now real, not a
placeholder, and verification is fail-closed. The signing step requires only
MONSYS_MINISIGN_SECRET_KEY (an unencrypted key matching this pub); minisign -S
signs non-interactively so no password secret is needed.
@MalteKiefer
MalteKiefer merged commit ef79b0a into main Jul 16, 2026
10 of 11 checks passed
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