Sign in

Search issue titles and bodies, change requests and comments. Private repositories you cannot read are not included.
vole
Clone

Read only, no credentials:

https://rodent.software/vole.git

Write over SSH with a registered key:

git clone ssh://git@rodent.software:2222/vole.git

A free and open-source forge that lives in your Git repo.

#6

P0: Make the audit log trustworthy

closed · opened by keenan 13d ago

keenan opened this 13d ago

Why

The audit log is a stated security promise, but every writer treats failure as best-effort and several do not sanitize fields:

  • server/web/audit.go
  • forge/identity/identity.go
  • forge/access/store.go
  • server/hook/hook.go
  • server/ssh/admin.go

Files are created 0644; identity key comments and LFS lock paths can inject tabs/newlines into columns.

Scope

  • Centralize audit formatting and sanitize tabs/newlines in every field.
  • Create audit files 0600 with O_APPEND.
  • Detect write failures and log them loudly.
  • For privileged admin/ACL/account changes, fail closed (or require an explicit emergency override).
  • Add a startup/health check for audit writability and ownership.
  • Document shipping the audit log off-host; local append-only is not tamper-evident.

Acceptance

  • A read-only or full audit filesystem causes privileged changes to fail or emit a high-severity alert, never a silent success.
  • Field-injection tests pass.

Comments (2)

keenan 13d ago

Plan for this P0 item, in the order I will land it:

  1. One writer. New internal/audit package: a single canonical record timestamp, action, actor, subject, detail, every field escaped (\t, \n, \r, other C0 controls) in one place so a key comment, an LFS lock path, or an admin argument cannot forge a column. One write(2) per line so O_APPEND keeps concurrent writers whole, and secure.OpenAppend keeps the file at 0600 including on upgrade from 0644. Errors are returned, never dropped.
  2. Every writer uses it. server/web/audit.go, forge/identity, forge/session, forge/access, server/hook, server/ssh/admin.go and core/lfs/lockstore.go all render through it, with named actions instead of free-form lines. The log’s column shape changes; the file is append-only, so old lines stay as they are and the CR body will say so.
  3. Fail closed for privileged changes, and fail before the change. identity.mutate already has a chokepoint: the audit line moves inside the transaction, before Commit. The same for ACL writes, session create/revoke, admin commands, and the hook’s admin overrides, where failing closed means rejecting the push. Over-recording is acceptable; under-recording is not, so no audit line is written after a mutation succeeds.
  4. Loud, not silent. The paths that stay best-effort (anonymous web writes) emit a high-severity log line naming the action and the account.
  5. Startup check. Opening an instance verifies the audit path is creatable, 0600, owned by the service account, and actually appendable, and refuses to start otherwise.
  6. Regression tests per bullet, each failing before its fix: field injection through key comments, lock paths, admin argv and repository names; 0600 on create and on upgrade; an unwritable sink makes an ACL and an account change fail with the database unchanged; concurrent appends keep whole lines.
  7. Docs: configuration, operations (shipping off-host, what fail-closed means, and the manual recovery path), and the architecture trust model.

No configuration knob turns the audit log off or back to best-effort: an override is exactly what an attacker with config access would set. Privileged changes fail instead.

keenan 13d ago

Implemented in #52 (branch audit-log-trustworthy, head b1499c7). All seven checklist items are covered, and every one has a test that failed before the change. No config knob was added: privileged changes fail closed instead.

The log format changes from free-form lines to timestamp, action, actor, subject, detail; the file is append-only so existing lines are untouched. docs/operations.md#audit-log documents the columns and the fail-closed behaviour.

./vole workflow run verify passes. The request needs an approval from another account before it can land.