diff --git a/AGENTS.md b/AGENTS.md index 6b18b6a..5042e03 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -38,6 +38,7 @@ All production code lives under `de.hoennig.gittally`, with sub-packages `comman - Nothing is scheduled during CLI runs or tests: the watcher poll loop and metrics sampling start only via an explicit `start()` in the `server` profile. - Builds run detached in worktrees under `.git/gittally/worktrees/`; the primary checkout is never used for builds; never assume a single running build. - When config keys change, three places must stay in sync: the `GitTallyConfig` data classes, the `InitCommand` templates, and `docs/configuration.md`. +- Build config is layered via `ConfigLoader.loadForWorktree`: the build worktree's `.gittally.yml` overrides `.git`/project for build-specific keys, but the pinned set — secrets/server-side sections (`git`, `gitea`, `server`) and the docker sandbox policy (`docker.enabled`, `docker.network`) — is stripped from the worktree layer and always comes from `.git`/primary. A branch must never be able to disable its container, change its network, or reach credentials via its committed config. - Web UI: server-rendered Thymeleaf plus one hand-written `static/gittally.js` — no SPA framework, no frontend build pipeline; every fetch has a timeout and an explicit error badge; `UiFormats` and `gittally.js` must produce identical display formats. - Git and Docker access shells out to the CLIs (`GitCommandRunner`, `docker`) — no JGit, no Docker SDK. diff --git a/docs/configuration.md b/docs/configuration.md index 16c990a..a618928 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -1,6 +1,6 @@ # GitTally Configuration Reference -GitTally is configured via YAML files. Settings are merged from two sources in order — later layers override earlier ones. +GitTally is configured via YAML files. Settings are merged from several sources in order — later layers override earlier ones. ## Config File Locations @@ -8,9 +8,29 @@ GitTally is configured via YAML files. Settings are merged from two sources in o |--------------------------|----------------------------|------------------|----------------------------------------------| | Project config | `.gittally.yml` | Yes | Shared team settings | | Repo installation config | `.git/gittally/.gittally.yml` | No | Machine- or user-specific overrides, secrets | +| Build worktree config | `.gittally.yml` of the built commit | Yes | Per-branch build settings (build layer only) | The repo install config (`.git/gittally/.gittally.yml`) wins on any key present in both files. Typically used to set `git.token` and `git.account` without committing them. +### Per-branch build settings from the worktree + +When a branch builds, its build config is resolved with an extra layer: the `.gittally.yml` +committed on the branch being built (read from its build worktree) overrides the two layers +above, giving the precedence **worktree > repo install > project**. So a branch can change its +own `buildCommand`, `cleanCommand`, `artifactDirs`, log file names, `autoBuild`, and +`docker.image`/`dockerfile`/`context`/`env`. + +This layer applies **only** to the build itself. A pinned set is always taken from the repo +install/project config and can never be set from the worktree: + +- secrets and server-side settings: the whole `git`, `gitea`, and `server` sections; +- the container sandbox policy: `docker.enabled` and `docker.network`. + +This keeps a branch from disabling its own build container, changing its network mode, or +reaching credentials. Watcher decisions that happen before a build exists — `autoBuild` +scheduling and the `requirePullRequest` gate — are still read from the repo install/project +config, because there is no worktree at that point. + ## Inspect the Effective Config ```bash diff --git a/docs/prs/2026-07-08-PR#000-security-audit-hardening.md b/docs/prs/2026-07-08-PR#000-security-audit-hardening.md index 352b6d8..63ba8f2 100644 --- a/docs/prs/2026-07-08-PR#000-security-audit-hardening.md +++ b/docs/prs/2026-07-08-PR#000-security-audit-hardening.md @@ -92,20 +92,20 @@ Raw build output may contain secrets echoed by build scripts; `/api/system` expo #### TODO 6 — Layer build config over the worktree, with secrets and sandbox pinned to `.git` -Intended behavior (a design change, not just a security fix): a branch should be built with its own build settings, so `.gittally.yml` from the **build worktree** (the commit being built) overrides the `.git`/primary config — except for a pinned set that a branch must never control. +A branch is built with its own build settings: `.gittally.yml` from the **build worktree** (the commit being built) overrides the `.git`/primary config — except for a pinned set that a branch must never control. **Implemented in this PR.** -- [ ] Add a worktree config layer for builds: when resolving `branchConfig` for a build ([`BuildExecutor.kt:369-375`](../../src/main/kotlin/de/hoennig/gittally/build/BuildExecutor.kt)), merge the worktree's `.gittally.yml` on top of the primary/`.git` config, worktree winning. -- [ ] **Pin these keys to `.git`/primary — never overridable from the worktree:** `git.*` (secrets), `gitea.*`, `server.*`, and the sandbox policy `docker.enabled` and `docker.network`. A branch must not be able to disable its own container or change its network mode. -- [ ] Leave these worktree-overridable: `buildCommand`, `cleanCommand`, `artifactDirs`, `stdoutLog`/`stderrLog`, `autoBuild`, and `docker.image`/`dockerfile`/`context`/`env`. -- [ ] Fix the precedence in [`ConfigLoader.loadRaw` (`ConfigLoader.kt:44-48`)](../../src/main/kotlin/de/hoennig/gittally/config/ConfigLoader.kt): for the build layer the order is worktree > `.git` > project, whereas today `.git` overlays project and the worktree is never read at all. -- [ ] Enforce the pinned set in code (strip pinned keys from the worktree layer before merging) and assert it in `AGENTS.md`, so the boundary is explicit rather than implicit. +- [x] Worktree config layer for builds via [`ConfigLoader.loadForWorktree`](../../src/main/kotlin/de/hoennig/gittally/config/ConfigLoader.kt), wired into the two build-time config consumers ([`BuildExecutor.branchConfig`](../../src/main/kotlin/de/hoennig/gittally/build/BuildExecutor.kt), [`FileArtifactStore.branchConfig`](../../src/main/kotlin/de/hoennig/gittally/artifacts/FileArtifactStore.kt)) — both already hold the prepared worktree path. Precedence is worktree > `.git` > project. +- [x] **Pinned to `.git`/primary — stripped from the worktree layer:** `git`, `gitea`, `server` (secrets + server-side), and the sandbox policy `docker.enabled`/`docker.network`. A branch cannot disable its container or change its network mode. +- [x] Worktree-overridable: `buildCommand`, `cleanCommand`, `artifactDirs`, `stdoutLog`/`stderrLog`, `autoBuild`, and `docker.image`/`dockerfile`/`context`/`env`. +- [x] Pinned set enforced in code (`ConfigLoader.stripPinned`), documented in `docs/configuration.md`, and asserted as an invariant in `AGENTS.md`. +- [ ] **Deferred:** `autoBuild` scheduling and `requirePullRequest` are still read from the primary config, not the worktree — the watcher evaluates them *before* a build (and thus a worktree) exists. Sourcing them per-branch would need the watcher to read the branch's committed config directly (e.g. via `git show :.gittally.yml`); out of scope here. **Background.** The build command is executed via `bash -c "$3"` inside the build container ([`DockerBuildRunner.kt:285`](../../src/main/kotlin/de/hoennig/gittally/build/DockerBuildRunner.kt)). Letting a branch define its own `buildCommand` is not a new risk — a CI already runs arbitrary code from that commit; the container is the sandbox. -The real escalation is a branch turning the sandbox **off**: if the worktree could set `docker.enabled: false` (or host `docker.network`), the build would run natively on the host. -Secrets are already safe from the build process (the Gitea token is used only by the server/watcher and is never placed in the build environment — `runCommand` passes only `mapOf("branch" to ...)`), and stay that way as long as `git.*`/`gitea.*` are excluded from the worktree layer. -Current state (verified): all build config is loaded from the primary checkout via `configLoader.load(build.workingDir)`, the worktree's `.gittally.yml` is never consulted, and `.git` config takes precedence over the committed project config — so this is a real feature, and it reverses the assumption the rest of this audit was written under. +The real escalation is a branch turning the sandbox **off**: if the worktree could set `docker.enabled: false` (or host `docker.network`), the build would run natively on the host — which is why those two keys are pinned. +Secrets are also safe from the build process (the Gitea token is used only by the server/watcher and is never placed in the build environment — `runCommand` passes only `mapOf("branch" to ...)`), and the whole `git`/`gitea`/`server` sections are stripped from the worktree layer as defense in depth. +Before this PR all build config was loaded from the primary checkout via `configLoader.load(build.workingDir)`, the worktree's `.gittally.yml` was never consulted, and `.git` took precedence over the committed project config — so this both adds the per-branch feature and reverses that assumption. ### Low / defense-in-depth diff --git a/src/main/kotlin/de/hoennig/gittally/artifacts/FileArtifactStore.kt b/src/main/kotlin/de/hoennig/gittally/artifacts/FileArtifactStore.kt index 97f18dc..d9018eb 100644 --- a/src/main/kotlin/de/hoennig/gittally/artifacts/FileArtifactStore.kt +++ b/src/main/kotlin/de/hoennig/gittally/artifacts/FileArtifactStore.kt @@ -138,7 +138,7 @@ class FileArtifactStore( log.warn("build {} has no workspace; storing only its logs", build.artifactKey) return } - for (artifactDir in branchConfig(build.branch).artifactDirs) { + for (artifactDir in branchConfig(build.branch, workspace).artifactDirs) { if (artifactDir.isBlank()) { continue } @@ -159,8 +159,12 @@ class FileArtifactStore( "reports/$artifactDir" } - private fun branchConfig(branch: String): BranchConfig { - val branches = configLoader.load(workingDir).branches + /** The build config for [branch], with the build [workspace]'s `.gittally.yml` layered on top (see [ConfigLoader.loadForWorktree]). */ + private fun branchConfig( + branch: String, + workspace: Path, + ): BranchConfig { + val branches = configLoader.loadForWorktree(workingDir, workspace).branches return branches[branch] ?: branches["default"] ?: BranchConfig() } diff --git a/src/main/kotlin/de/hoennig/gittally/build/BuildExecutor.kt b/src/main/kotlin/de/hoennig/gittally/build/BuildExecutor.kt index fb51402..1cf9898 100644 --- a/src/main/kotlin/de/hoennig/gittally/build/BuildExecutor.kt +++ b/src/main/kotlin/de/hoennig/gittally/build/BuildExecutor.kt @@ -178,7 +178,7 @@ class BuildExecutor( build: ActiveBuild, workspace: Path, ): Int { - val branchConfig = branchConfig(build.runningBuild.branch, build.workingDir) + val branchConfig = branchConfig(build.runningBuild.branch, build.workingDir, workspace) val stagingDir = build.runningBuild.stagingDir Files.newOutputStream(stagingDir.resolve(branchConfig.stdoutLog)).use { stdoutLog -> Files.newOutputStream(stagingDir.resolve(branchConfig.stderrLog)).use { stderrLog -> @@ -366,11 +366,13 @@ class BuildExecutor( } } + /** The build config for [branch], with the build [worktree]'s `.gittally.yml` layered on top (see [ConfigLoader.loadForWorktree]). */ private fun branchConfig( branch: String, workingDir: Path, + worktree: Path, ): BranchConfig { - val branches = configLoader.load(workingDir).branches + val branches = configLoader.loadForWorktree(workingDir, worktree).branches return branches[branch] ?: branches["default"] ?: BranchConfig() } diff --git a/src/main/kotlin/de/hoennig/gittally/config/ConfigLoader.kt b/src/main/kotlin/de/hoennig/gittally/config/ConfigLoader.kt index 5866bb1..088e3e3 100644 --- a/src/main/kotlin/de/hoennig/gittally/config/ConfigLoader.kt +++ b/src/main/kotlin/de/hoennig/gittally/config/ConfigLoader.kt @@ -19,8 +19,33 @@ class ConfigLoader { .configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false) .configure(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS, false) - fun load(workingDir: Path = Paths.get(".")): GitTallyConfig { - val raw = loadRaw(workingDir) + fun load(workingDir: Path = Paths.get(".")): GitTallyConfig = toConfig(loadRaw(workingDir)) + + /** + * Config for building a branch in [worktreeDir]: the worktree's `.gittally.yml` + * (the committed config of the branch being built) overrides the primary/`.git` + * config, giving the precedence worktree > `.git` > project. So a branch controls + * its own build settings (`buildCommand`, `cleanCommand`, `artifactDirs`, + * `docker.image`/`env`, …). + * + * The [pinned][stripPinned] keys are the exception: secrets (`git`), `gitea`/`server` + * settings, and the docker sandbox policy (`docker.enabled`/`docker.network`) always + * come from `.git`/primary — a branch must never be able to disable its own container, + * change its network mode, or reach the credentials. They are stripped from the + * worktree layer before it is merged, so a worktree cannot set them at all. + * + * With no worktree `.gittally.yml` this is identical to [load]. + */ + fun loadForWorktree( + workingDir: Path, + worktreeDir: Path, + ): GitTallyConfig { + val primary = loadRaw(workingDir) + val worktree = stripPinned(loadFile(worktreeDir.resolve(".gittally.yml").toFile())) + return toConfig(deepMerge(primary, worktree)) + } + + private fun toConfig(raw: Map): GitTallyConfig { val config = if (raw.isEmpty()) { GitTallyConfig() @@ -30,6 +55,32 @@ class ConfigLoader { return defaultPublicBaseUrl(config) } + /** + * Removes the keys a build worktree must never override: the secret/server-side + * top-level sections and the per-branch docker sandbox policy. See [loadForWorktree]. + */ + @Suppress("UNCHECKED_CAST") + private fun stripPinned(worktree: Map): Map { + if (worktree.isEmpty()) { + return worktree + } + val result = worktree.toMutableMap() + PINNED_TOP_LEVEL_KEYS.forEach { result.remove(it) } + val branches = result["branches"] as? Map + if (branches != null) { + result["branches"] = + branches.mapValues { (_, value) -> + val branch = value as? Map ?: return@mapValues value + val docker = branch["docker"] as? Map ?: return@mapValues branch + val strippedDocker = docker.toMutableMap().apply { PINNED_DOCKER_KEYS.forEach { remove(it) } } + branch.toMutableMap().apply { + if (strippedDocker.isEmpty()) remove("docker") else put("docker", strippedDocker) + } + } + } + return result + } + /** Legacy default: an empty `server.publicBaseUrl` becomes `https:///`. */ private fun defaultPublicBaseUrl(config: GitTallyConfig): GitTallyConfig { if (config.server.publicBaseUrl.isNotBlank() || @@ -88,4 +139,12 @@ class ConfigLoader { } return result } + + companion object { + /** Top-level sections a build worktree must never override: secrets and server-side settings. */ + private val PINNED_TOP_LEVEL_KEYS = setOf("git", "gitea", "server") + + /** Per-branch `docker` keys the worktree must never override: the sandbox policy. */ + private val PINNED_DOCKER_KEYS = setOf("enabled", "network") + } } diff --git a/src/test/kotlin/de/hoennig/gittally/config/ConfigLoaderTest.kt b/src/test/kotlin/de/hoennig/gittally/config/ConfigLoaderTest.kt index fd18af3..daee18d 100644 --- a/src/test/kotlin/de/hoennig/gittally/config/ConfigLoaderTest.kt +++ b/src/test/kotlin/de/hoennig/gittally/config/ConfigLoaderTest.kt @@ -150,6 +150,112 @@ class ConfigLoaderTest : FunSpec() { loader.load(dir).server.publicBaseUrl shouldBe "" } + test("loadForWorktree lets the worktree override build config (worktree > .git > project)") { + val dir = Files.createTempDirectory("gittally-test") + dir.resolve(".gittally.yml").toFile().writeText( + """ + branches: + default: + buildCommand: from-project + """.trimIndent(), + ) + dir.resolve(".git/gittally").toFile().mkdirs() + dir.resolve(".git/gittally/.gittally.yml").toFile().writeText( + """ + branches: + default: + buildCommand: from-git + """.trimIndent(), + ) + val worktree = Files.createTempDirectory("gittally-worktree") + worktree.resolve(".gittally.yml").toFile().writeText( + """ + branches: + default: + buildCommand: from-worktree + """.trimIndent(), + ) + loader.loadForWorktree(dir, worktree).branches["default"]!!.buildCommand shouldBe "from-worktree" + } + + test("loadForWorktree falls back to .git over project when the worktree sets nothing") { + val dir = Files.createTempDirectory("gittally-test") + dir.resolve(".gittally.yml").toFile().writeText( + """ + branches: + default: + buildCommand: from-project + """.trimIndent(), + ) + dir.resolve(".git/gittally").toFile().mkdirs() + dir.resolve(".git/gittally/.gittally.yml").toFile().writeText( + """ + branches: + default: + buildCommand: from-git + """.trimIndent(), + ) + val worktree = Files.createTempDirectory("gittally-worktree") + loader.loadForWorktree(dir, worktree).branches["default"]!!.buildCommand shouldBe "from-git" + } + + test("loadForWorktree pins secrets and the docker sandbox policy to .git, but allows docker.image") { + val dir = Files.createTempDirectory("gittally-test") + dir.resolve(".git/gittally").toFile().mkdirs() + dir.resolve(".git/gittally/.gittally.yml").toFile().writeText( + """ + git: + token: real-secret + server: + port: 9000 + branches: + default: + docker: + enabled: true + network: host + image: trusted-image + """.trimIndent(), + ) + val worktree = Files.createTempDirectory("gittally-worktree") + worktree.resolve(".gittally.yml").toFile().writeText( + """ + git: + token: stolen + server: + port: 1234 + branches: + default: + docker: + enabled: false + network: none + image: attacker-image + """.trimIndent(), + ) + val config = loader.loadForWorktree(dir, worktree) + // pinned: never taken from the worktree + config.git.token shouldBe "real-secret" + config.server.port shouldBe 9000 + config.branches["default"]!!.docker.enabled shouldBe true + config.branches["default"]!!.docker.network shouldBe "host" + // overridable: the worktree wins + config.branches["default"]!!.docker.image shouldBe "attacker-image" + } + + test("loadForWorktree without a worktree config equals load") { + val dir = Files.createTempDirectory("gittally-test") + dir.resolve(".gittally.yml").toFile().writeText( + """ + gitea: + owner: my-org + branches: + default: + buildCommand: ./mvnw test + """.trimIndent(), + ) + val worktree = Files.createTempDirectory("gittally-worktree") + loader.loadForWorktree(dir, worktree) shouldBe loader.load(dir) + } + test("toYaml serializes GitTallyConfig with all sections") { val yaml = loader.toYaml(GitTallyConfig()) yaml shouldContain "server:"