werkator/docs/prs/2026-09-11-PR#55-security-hardening.md

WARNING: This document describes only the change applied in this PR. It may already be outdated once the next PR is merged. Historic PR-documentation is not maintained along with new PRs — treat it as a snapshot, not as current documentation.

The Problem

A security audit found six ways a build could reach past the boundaries Werkator sets for it. Five of them are holes in defences the code already claims to have.

Werkator states its trust model in several places. stripPinned exists so a branch cannot switch off its own sandbox. requirePullRequest is a gate for building what is not fully trusted. The Docker and werkdock runners mask .git/werkator/ with an empty tmpfs so a build never sees the git token. ADR 0008 exists because builds are expected to run isolated. Each finding below is a place where one of these statements did not hold.

The objection that whoever may push to a built branch has the build host anyway does not carry. It is contradicted by the code's own defences, and it breaks down in three concrete ways: one instance serves several repositories (ADR 0009), so a pusher to one is not authorised on another; artifacts of a public repository are downloadable by anyone, so the audience of a leak is the internet, not the pusher; and the victim of the artifact-origin finding is the operator, whose browser holds the control token.

Non-Goals

• No separate origin or host for artifact serving; the sandbox header solves the same problem without new DNS, certificates and broken permalinks.
• No disposable per-build Docker daemon. The socket becomes opt-in here; isolating the builds that genuinely need a daemon is deferred.
• No change to the artifact-collection order in BuildExecutor. The terminal status is still recorded before artifacts are collected, and the escape case takes the status back afterwards instead of reordering the lifecycle.
• No review of the trust model as such: this PR restores what the code promises, it does not redefine it.

The Scenarios

Feature: a build stays inside the boundaries set for it

Background

• The branch layer is the .werkator.yml committed on a branch; it describes that branch's build and loses its pinned keys (ConfigLoader.stripPinned).
• Staging is the per-build directory the captured output is written to, later copied into the artifact store.
• Collection is the copying of a build's declared artifact directories into the store; it runs in the server process on the host, not in the build's sandbox.

Scenario#55.01: An artifact page cannot act as the instance

So that a report a build wrote cannot use the operator's session against the instance that serves it.

• Given a build stored an HTML page as an artifact
• When an operator opens that page
• Then the response carries Content-Security-Policy: sandbox allow-scripts
  • and the document runs on an opaque origin, reaching neither the control token in localStorage nor the JSON API
  • and its own scripts still run, so an interactive report keeps working
  • and the response carries X-Content-Type-Options: nosniff
  • and the same holds for every path to the same bytes, the permanent /branches/… URL included

Verified by

• ArtifactFileControllerTest: an html artifact is sandboxed onto its own origin, away from the control token
• ArtifactFileControllerTest: every artifact file is sandboxed, not only the pages a build calls index.html
• ArtifactFileControllerTest: the permanent URL is sandboxed like every other way to the same bytes

Scenario#55.02: A build gets a Docker daemon only when it says it needs one

So that the way around the container is taken deliberately, by the repository, and not handed to every build by default.

• Given a repository configures docker.enabled: true
• When a build runs without docker.socket
• Then no Docker socket is mounted into the build container
• When a build runs with docker.socket: true
• Then the socket is mounted as before
• And a branch that sets docker.socket in its own committed configuration is ignored, because the key is pinned
• And whether the daemon is rootless still decides the ownership repair, independently of the socket

Verified by

• DockerBuildRunnerTest: without docker.socket the daemon stays outside the build container
• DockerBuildRunnerTest: a build without the socket still follows the rootless daemon's ownership rules
• ConfigLoaderTest: loadForWorktree pins secrets and the docker sandbox policy to .git, but allows docker.image

Scenario#55.03: A werkdock build cannot write the primary checkout

So that a build cannot set the configuration the next build is trusted to read.

• Given a build runs in a werkdock sandbox
• When the sandbox is assembled
• Then the repository directory is mounted read-only
  • and the branch's worktree is mounted read-write
  • and that worktree's git admin directory is mounted read-write
  • and the committed .werkator.yml at the repository root is therefore not writable, which is the layer the pinned settings are read from

Verified by

• WerkdockBuildRunnerTest: mounts the primary checkout read-only, the workspace read-write
• WerkdockBuildRunnerTest: assembles the exact werkdock run command for a loaded image

Scenario#55.04: Artifact collection does not read for the build what the build cannot read

So that a build cannot use the server as its reader, and a public repository cannot be turned into a publisher of host files.

• Given a build's artifact directory holds a symlink
• When the link resolves inside the collected directory
• Then it is copied as before
• When the link leaves the collected directory, or dangles
• Then the collection is aborted and nothing of that build is stored
  • and the build is recorded as FAILED even though its commands succeeded
  • and the link's target is untouched

Verified by

• FileArtifactStoreTest: a symlink out of the artifact directory is refused and nothing is stored
• FileArtifactStoreTest: a symlink inside the artifact directory is stored like the file it names
• FileArtifactStoreTest: a dangling symlink is refused rather than guessed at
• BuildExecutorArtifactIntegrationTest: a build whose artifacts point out of the workspace ends up FAILED with nothing stored

Scenario#55.05: The pinned settings hold whatever the branch layer looks like

So that a branch cannot switch off a sandbox by writing a value that removes it instead of setting it.

• Given a host requires containers (docker.enabled: true)
• When a branch writes docker: null, or a definition as a scalar
• Then the load is refused, naming the offending key and the shape it expects
• When a branch writes builds.<name>: null
• Then that is accepted, because it says this branch runs no such build
• When a branch empties builds.default and defines a build of its own
• Then that build still carries the host's sandbox policy, taken from the host's builds.default

Verified by

• ConfigLoaderTest: a branch that writes a pinned block as null is refused, not silently obeyed
• ConfigLoaderTest: a branch that writes a definition as a scalar is refused
• ConfigLoaderTest: emptying builds.default does not drop the sandbox policy of the branch's own builds
• ConfigLoaderTest: a branch whose layer lacks the predecessor loses only its follow-up

Scenario#55.06: A build's log file is a name, not a path

So that a build cannot have the server truncate and overwrite a file on the host with its output.

• Given a build definition sets stdoutLog or stderrLog
• When the value is a plain file name
• Then the log is written inside the build's staging directory as before
• When the value is absolute, climbs out with .., names a subdirectory, or is blank
• Then the build fails before any file is opened, and the message names the key

Verified by

• BuildLogNamesTest
• BuildExecutorArtifactIntegrationTest: a build naming a host path as its log file fails without writing that file

The Solution

Artifacts on their own origin. ArtifactFileController.respond — the single place every artifact byte passes — adds Content-Security-Policy: sandbox allow-scripts and X-Content-Type-Options: nosniff. allow-scripts keeps interactive reports working; allow-same-origin must never join it, as the two together dissolve the sandbox. A separate origin would be stronger still, and is a non-goal here: it costs DNS, certificates and every existing permalink, including the target_url already recorded in Gitea.

The Docker socket is opt-in. New docker.socket, default false, pinned alongside docker.enabled and docker.network. The socket is still located either way, because whether the daemon is rootless decides the in-container ownership repair — that is a property of the host, not a permission of the build.

The primary checkout is read-only under werkdock. Only the branch's worktree and its git admin directory stay writable, which is what a build needs. The mount order is unchanged; the base mount merely lost its write permission.

Collection stops at the edge of what it collects. Files.copy follows symlinks, and collection runs on the host outside the build's sandbox, so a link was enough to have Werkator read a file the build could not open. Links resolving inside the collected directory are copied as before; one that leaves it raises ArtifactEscapeException, the incoming directory is discarded, and BuildExecutor takes the terminal status back to FAILED. That late correction is deliberate: collection runs after the status is recorded, and reordering the lifecycle would touch the worktree pruning and the "still finishing" view that rely on the current order.

Pinning is enforced, not only subtracted. checkLayerShape refuses a branch layer that writes a non-map where a block belongs, naming the key — ignoring it would leave a line doing something other than what it says, the same reasoning as checkTriggerBlocks. enforcePinned then writes the pinned values back from the host's layers after merging, falling back to the host's builds.default, so the pinning holds by what the merged configuration says rather than by what was removed from the branch layer. builds.<name>: null keeps its established meaning.

Log names are names. BuildLogNames.requireSimple rejects absolute, climbing, nested and blank names before any stream is opened.

Open Questions

• The artifact sandbox forbids a report from calling Werkator's own API. No report is known to do this; if one ever should, it needs its own decision rather than a wider header.
• docker.socket makes the privilege visible but does not reduce it: a build that sets it is still as privileged as the daemon. The disposable daemon is the actual fix (see Follow-up PRs).
• A build failing at collection reports success to Gitea first and failure a moment later. Correct in substance, briefly confusing in the UI.

Additional Changes

• AGENTS.md, docs/configuration.md and the architecture skill describe the new pinned key and the two-part pinning.
• The init templates document docker.socket and the log-name rule.
• Version bumped to 1.10.0 with its release-notes entry.

Prerequisite PRs

• PR#54 — the branch starts from it; RFC 0003 D8/D9 define what a private repository gives out, which Scenario#55.01 and #55.04 build on.

Follow-up PRs

• A disposable Docker daemon per build, so that builds needing a daemon stop needing the host's (audit finding 3, the part left open here).
• vm4006 needs docker.socket: true for Werkator's own Testcontainers build before it is updated to this version.