Range-diff !476
back to !476 wiki: Architecture fixes; review gaps filed as #273-#282
1: 1419181 ! 1: fc5b1b6 wiki: Architecture overview names diagrams; titles drop their numbers
@@ Metadata
Author: Christian Cleberg <hello@cleberg.net>
## Commit message ##
- wiki: Architecture overview names diagrams; titles drop their numbers
+ wiki: Architecture overview names diagrams; titles drop their numbers; gaps filed
- A link to a non-page file resolves to the page route and 404s. A title
- starting "5. " renders as an ordered list numbered from 1; the sidebar
- already carries the numbers.
+ A link to a non-page file resolves to the page route and 404s (#283).
+ A title starting "5. " renders as an ordered list numbered from 1; the
+ sidebar already carries the numbers. The review's unfiled gaps are now
+ #273-#282; Known gaps and the controls matrix cite them.
- Ref #272
+ Ref #272, #283
## .gitbay/wiki/Architecture/00-Overview.org ##
@@ .gitbay/wiki/Architecture/00-Overview.org: the =ReadOnly= count from the =ReadOnly: true= literals in
@@ .gitbay/wiki/Architecture/09-Controls.org
One row per control an auditor typically asks about. *Status*: =in
place= (implemented and cited), =partial= (implemented with a stated
+@@ .gitbay/wiki/Architecture/09-Controls.org: chapter names of OWASP ASVS 4.0 where one fits.
+ | Brute-force limit on SSH auth | in place | 10 failures a minute per IP (=internal/sshd/ratelimit.go=) |
+ | Account enumeration resistance at login | in place | uniform response (=internal/control/loginlink.go=) |
+ | Session cookie flags | in place | HttpOnly, SameSite=Lax, Secure with TLS (=internal/httpd/accounts.go=) |
+-| Session lifetime | partial | 7 days absolute, no idle timeout |
+-| Credential expiry | partial | API tokens optional; SSH and deploy keys none |
++| Session lifetime | partial | 7 days absolute, no idle timeout (#276) |
++| Credential expiry | partial | API tokens optional; SSH and deploy keys none (#277) |
+ | Revocation takes effect immediately | gap | removed SSH key keeps open connections (#256) |
+ | Delegation bounded by the delegating credential | gap | expiring tokens can mint lasting credentials (#257) |
+
+@@ .gitbay/wiki/Architecture/09-Controls.org: chapter names of OWASP ASVS 4.0 where one fits.
+ | Control | Status | Evidence |
+ |---------------------------------------------+----------+------------------------------------------------------------------|
+ | TLS for all authenticated HTTP | in place | ACME or certificate files; HSTS |
+-| Secrets encrypted at rest | gap | CI secrets, webhook secrets, mirror tokens stored in clear ([[file:06-Data-and-Cryptography.org][6]]) |
++| Secrets encrypted at rest | gap | CI secrets, webhook secrets, mirror tokens stored in clear (#273) |
+ | Secrets kept out of argv, logs and output | in place | =ReadsStdin=, pruned audit argv, write-only secret commands |
+-| Local backups encrypted | gap | tar.gz in clear; offsite copy encrypted by restic |
++| Local backups encrypted | gap | tar.gz in clear; offsite copy encrypted by restic (#274) |
+ | Data retention configurable | in place | =[retention]= (=internal/config/config.go=) |
+ | User data export | in place | =account export= |
+
+@@ .gitbay/wiki/Architecture/09-Controls.org: chapter names of OWASP ASVS 4.0 where one fits.
+ |---------------------------------------------+----------+------------------------------------------------------------------|
+ | Security-relevant writes audited | in place | every successful mutating command (=control.go=) |
+ | Authentication failures audited | in place | =auth.failed=, =auth.throttled= |
+-| Denied attempts audited | gap | refused commands are not recorded |
+-| Audit log tamper resistance | gap | same database, writable by the daemon user |
++| Denied attempts audited | gap | refused commands are not recorded (#275) |
++| Audit log tamper resistance | gap | same database, writable by the daemon user (#275) |
+
+ ** Communications and integrations (V9, V10, V12)
+
+ | Control | Status | Evidence |
+ |---------------------------------------------+----------+------------------------------------------------------------------|
+-| SSRF protection on user-supplied URLs | partial | webhooks at save and connect; mirrors at save only |
++| SSRF protection on user-supplied URLs | partial | webhooks at save and connect; mirrors at save only (#279) |
+ | Webhook payload integrity | in place | HMAC-SHA256 header |
+-| SMTP credentials protected in transit | partial | STARTTLS opportunistic; Go refuses PLAIN auth without TLS to a remote host |
++| SMTP credentials protected in transit | partial | STARTTLS opportunistic (#280); Go refuses PLAIN auth without TLS to a remote host |
+ | Upload size limits | in place | per-owner storage quota at push (=internal/sshd/sshd.go=); API body 1 MiB |
+
+ ** CI and build isolation
## .gitbay/wiki/Architecture/10-Known-Gaps.org ##
@@
@@ .gitbay/wiki/Architecture/10-Known-Gaps.org
Open weaknesses. Issues on krz/gitbay are public; this page gives the
title and the consequence, not a reproduction. The current list is the
+@@ .gitbay/wiki/Architecture/10-Known-Gaps.org: what the 2026-09-27 review found; remove a row when its issue closes.
+ | #260 | CI network | Builds share the runner's source address; no egress policy | medium |
+ | #261 | Various | Migration foreign-key check after commit; three web writes bypass dispatch; documentation drift | medium |
+ | #262 | Availability | No limit on concurrent git pack generation | high |
++| #273 | Data at rest | CI secrets, webhook secrets and mirror tokens are stored in clear in SQLite | high |
++| #274 | Backups | The local backup archive is not encrypted | medium |
++| #275 | Audit | Refused writes are not audited; the audit table is writable by the daemon user | medium |
++| #276 | Sessions | Web sessions last 7 days with no idle timeout | low |
++| #277 | Credentials | SSH and deploy keys never expire | low |
++| #278 | Login links | =web login= over SSH skips the login-link rate limit | low |
++| #279 | SSRF | Mirror URLs are checked when saved, not when git connects | medium |
++| #280 | Mail | STARTTLS only when the relay offers it | medium |
++| #281 | TLS | No explicit minimum TLS version | low |
++| #282 | Hook socket | Anything that can open =hook.sock= can act as any user | medium |
+
+ Decisions already taken on these: #256 closes a removed key's
+ connections, running commands included; #257 refuses credential
+ creation from expiring tokens, records which token created each
+ credential, and makes =read= the default scope.
+
+-* Not yet filed
+-
+-Found during the 2026-09-27 review.
+-
+-| Area | Gap | Where |
+-|------------------+---------------------------------------------------------------------------------------+---------------------------------------------|
+-| Data at rest | CI secrets, webhook secrets and mirror tokens are stored in clear in SQLite | =build_secrets=, =webhooks=, =mirrors= |
+-| Backups | The local backup archive is not encrypted | =cmd/gitbayd/backup.go= |
+-| Audit | Refused commands are not audited; the audit table is writable by the daemon user | =internal/control/control.go= |
+-| Sessions | Web sessions have a 7-day absolute lifetime and no idle timeout | =internal/httpd/accounts.go= |
+-| Credentials | SSH and deploy keys never expire | =ssh_keys= |
+-| Login links | =web login= over SSH is not counted against the 5-per-hour login-link limit | =internal/control/web.go= |
+-| SSRF | Mirror URLs are checked when saved but not when git connects, so a DNS change can redirect a mirror to a private address | =internal/control/mirrorcmd.go= |
+-| Mail | STARTTLS is used only when the relay offers it | =internal/mail/mail.go= |
+-| TLS | Go defaults; no explicit minimum version | =cmd/gitbayd/main.go= |
+-| Hook socket | Any process that can open =hook.sock= can claim any user id; it relies on the data directory's permissions | =internal/hookd/hookd.go= |
+-
+ * Questions an auditor will ask that have no answer yet
+
+ | Question | Status |