Added worktree-layered build config: resolves .gittally.yml from the worktree for per-branch build settings, with precedence worktree > .git > project; pinned secrets, server-side keys, and sandbox policy to .git.
This commit is contained in:
@@ -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/<branchKey>`; 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.
|
||||
|
||||
|
||||
+21
-1
@@ -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
|
||||
|
||||
@@ -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 <branch>:.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
|
||||
|
||||
|
||||
@@ -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()
|
||||
}
|
||||
|
||||
|
||||
@@ -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()
|
||||
}
|
||||
|
||||
|
||||
@@ -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<String, Any?>): 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<String, Any?>): Map<String, Any?> {
|
||||
if (worktree.isEmpty()) {
|
||||
return worktree
|
||||
}
|
||||
val result = worktree.toMutableMap()
|
||||
PINNED_TOP_LEVEL_KEYS.forEach { result.remove(it) }
|
||||
val branches = result["branches"] as? Map<String, Any?>
|
||||
if (branches != null) {
|
||||
result["branches"] =
|
||||
branches.mapValues { (_, value) ->
|
||||
val branch = value as? Map<String, Any?> ?: return@mapValues value
|
||||
val docker = branch["docker"] as? Map<String, Any?> ?: 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://<nginx.serverName>/`. */
|
||||
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")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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:"
|
||||
|
||||
Reference in New Issue
Block a user