Header-only control token, masked secrets, loopback default (v0.9.9)
Finishes the small items of the security audit in docs/prs/2026-07-08-PR#000: TODO 3, 4 and 7. The three mutating endpoints of BuildsApiController no longer accept the control token as a `token` query parameter — only the X-GitTally-Token header, which the bundled UI has always used. URLs end up in access logs, proxy logs, browser history and Referer headers, and the token never expires, so a historical log capture would yield a valid credential. `config:print` masks git.token as `***` on both the raw and the --full path and names the new --show-secrets flag in a leading YAML comment, so the output stays parseable when piped. The setup script points at --show-secrets where it used to steer the operator to the plain token. `server.bindAddress` now defaults to 127.0.0.1: neither the UI nor the API authenticates read access, so reaching GitTally should require the host's reverse proxy. Existing .gittally.yml files keep their explicit value; the managed nginx container needs `0.0.0.0` set deliberately, which is noted in the release notes, docs/configuration.md and docs/deployment.md. Released as v0.9.9, which also carries the previous two commits. Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,74 @@
|
||||
package de.hoennig.gittally.commands
|
||||
|
||||
import de.hoennig.gittally.config.ConfigLoader
|
||||
import de.hoennig.gittally.config.GitConfig
|
||||
import de.hoennig.gittally.config.GitTallyConfig
|
||||
import io.kotest.core.spec.style.FunSpec
|
||||
import io.kotest.matchers.string.shouldContain
|
||||
import io.kotest.matchers.string.shouldNotContain
|
||||
import io.mockk.clearMocks
|
||||
import io.mockk.every
|
||||
import io.mockk.mockk
|
||||
|
||||
class ConfigPrintCommandTest : FunSpec() {
|
||||
private val yamlWriter = ConfigLoader()
|
||||
private val configLoader = mockk<ConfigLoader>()
|
||||
private val command = ConfigPrintCommand(configLoader)
|
||||
|
||||
private val rawConfig =
|
||||
mapOf<String, Any?>(
|
||||
"git" to mapOf("account" to "ci-user", "token" to "s3cr3t-token"),
|
||||
"server" to mapOf("port" to 18080),
|
||||
)
|
||||
|
||||
init {
|
||||
beforeEach {
|
||||
clearMocks(configLoader)
|
||||
every { configLoader.toYaml(any()) } answers { yamlWriter.toYaml(firstArg()) }
|
||||
every { configLoader.loadRaw() } returns rawConfig
|
||||
every { configLoader.load() } returns GitTallyConfig(git = GitConfig(account = "ci-user", token = "s3cr3t-token"))
|
||||
command.full = false
|
||||
command.showSecrets = false
|
||||
}
|
||||
|
||||
test("masks the git token by default") {
|
||||
val output = captureConsole { command.run() }.stdout
|
||||
|
||||
output shouldNotContain "s3cr3t-token"
|
||||
output shouldContain "***"
|
||||
output shouldContain "# git.token is masked"
|
||||
output shouldContain "account: \"ci-user\""
|
||||
}
|
||||
|
||||
test("masks the git token with --full as well") {
|
||||
command.full = true
|
||||
|
||||
val output = captureConsole { command.run() }.stdout
|
||||
|
||||
output shouldNotContain "s3cr3t-token"
|
||||
output shouldContain "***"
|
||||
}
|
||||
|
||||
test("prints the git token with --show-secrets") {
|
||||
command.showSecrets = true
|
||||
|
||||
val output = captureConsole { command.run() }.stdout
|
||||
|
||||
output shouldContain "s3cr3t-token"
|
||||
output shouldNotContain "masked"
|
||||
}
|
||||
|
||||
test("prints the git token with --full --show-secrets") {
|
||||
command.full = true
|
||||
command.showSecrets = true
|
||||
|
||||
captureConsole { command.run() }.stdout shouldContain "s3cr3t-token"
|
||||
}
|
||||
|
||||
test("says nothing about masking when no token is configured") {
|
||||
every { configLoader.loadRaw() } returns mapOf("server" to mapOf("port" to 18080))
|
||||
|
||||
captureConsole { command.run() }.stdout shouldNotContain "masked"
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -175,7 +175,7 @@ class BuildsApiControllerTest : FunSpec() {
|
||||
runningBuild(liveLogFile).copy(branch = "fresh")
|
||||
|
||||
mockMvc
|
||||
.perform(post("/api/builds/restart").param("branch", "fresh").param("token", "secret"))
|
||||
.perform(post("/api/builds/restart").param("branch", "fresh").header(BuildsApiController.TOKEN_HEADER, "secret"))
|
||||
.andExpect(status().isAccepted)
|
||||
.andExpect(jsonPath("$.status").value("pending"))
|
||||
|
||||
@@ -187,7 +187,7 @@ class BuildsApiControllerTest : FunSpec() {
|
||||
every { gitService.originHeadCommit("gone", any()) } returns null
|
||||
|
||||
mockMvc
|
||||
.perform(post("/api/builds/restart").param("branch", "gone").param("token", "secret"))
|
||||
.perform(post("/api/builds/restart").param("branch", "gone").header(BuildsApiController.TOKEN_HEADER, "secret"))
|
||||
.andExpect(status().isNotFound)
|
||||
}
|
||||
|
||||
@@ -227,14 +227,30 @@ class BuildsApiControllerTest : FunSpec() {
|
||||
every { buildExecutor.cancel("unknown-key") } returns false
|
||||
|
||||
mockMvc
|
||||
.perform(post("/api/builds/known-key/cancel").param("token", "secret"))
|
||||
.perform(post("/api/builds/known-key/cancel").header(BuildsApiController.TOKEN_HEADER, "secret"))
|
||||
.andExpect(status().isAccepted)
|
||||
.andExpect(jsonPath("$.cancelled").value("known-key"))
|
||||
mockMvc
|
||||
.perform(post("/api/builds/unknown-key/cancel").param("token", "secret"))
|
||||
.perform(post("/api/builds/unknown-key/cancel").header(BuildsApiController.TOKEN_HEADER, "secret"))
|
||||
.andExpect(status().isNotFound)
|
||||
}
|
||||
|
||||
test("a token in the query string is not accepted — the header is the only way") {
|
||||
mockMvc
|
||||
.perform(post("/api/builds/restart").param("branch", "main").param("token", "secret"))
|
||||
.andExpect(status().isForbidden)
|
||||
mockMvc
|
||||
.perform(post("/api/builds/some-key/cancel").param("token", "secret"))
|
||||
.andExpect(status().isForbidden)
|
||||
mockMvc
|
||||
.perform(delete("/api/builds/some-key").param("token", "secret"))
|
||||
.andExpect(status().isForbidden)
|
||||
|
||||
verify(exactly = 0) { buildExecutor.startBuild(any(), any(), any()) }
|
||||
verify(exactly = 0) { buildExecutor.cancel(any()) }
|
||||
verify(exactly = 0) { repository.delete(any()) }
|
||||
}
|
||||
|
||||
test("cancel without token answers 403") {
|
||||
mockMvc
|
||||
.perform(post("/api/builds/some-key/cancel"))
|
||||
@@ -260,7 +276,7 @@ class BuildsApiControllerTest : FunSpec() {
|
||||
every { repository.delete("unknown-key") } returns false
|
||||
|
||||
mockMvc
|
||||
.perform(delete("/api/builds/unknown-key").param("token", "secret"))
|
||||
.perform(delete("/api/builds/unknown-key").header(BuildsApiController.TOKEN_HEADER, "secret"))
|
||||
.andExpect(status().isNotFound)
|
||||
|
||||
verify(exactly = 0) { artifactStore.prune(any()) }
|
||||
|
||||
Reference in New Issue
Block a user