From a4c995592f0effa4f1b98316419fd198c9500208 Mon Sep 17 00:00:00 2001 From: mhoennig Date: Tue, 11 Aug 2026 07:38:33 +0200 Subject: [PATCH] Header-only control token, masked secrets, loopback default (v0.9.9) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- build.gradle.kts | 2 +- docs/configuration.md | 15 +++- docs/deployment.md | 4 +- ...6-07-08-PR#000-security-audit-hardening.md | 12 +-- .../gittally/commands/ConfigPrintCommand.kt | 29 +++++++- .../hoennig/gittally/commands/InitCommand.kt | 6 +- .../hoennig/gittally/config/GitTallyConfig.kt | 7 +- .../gittally/server/BuildsApiController.kt | 15 ++-- src/main/resources/templates/releases.html | 16 ++++ .../commands/ConfigPrintCommandTest.kt | 74 +++++++++++++++++++ .../server/BuildsApiControllerTest.kt | 26 +++++-- tools/setup-gittally-instance | 3 +- 12 files changed, 180 insertions(+), 29 deletions(-) create mode 100644 src/test/kotlin/de/hoennig/gittally/commands/ConfigPrintCommandTest.kt diff --git a/build.gradle.kts b/build.gradle.kts index bb85656..7e9f089 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -11,7 +11,7 @@ plugins { group = "de.hoennig" // bump at least the patch version for every deployment, so the UI footer // (BuildProperties) and --version identify what is actually running -version = "0.9.8" +version = "0.9.9" java { toolchain { diff --git a/docs/configuration.md b/docs/configuration.md index 91dc56e..13fb289 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -38,6 +38,9 @@ java -jar build/libs/gittally.jar config:print # only explicitly set val java -jar build/libs/gittally.jar config:print --full # all values including defaults ``` +`git.token` is masked as `***` by default, so the output can safely be shared or pasted. +Add `--show-secrets` to print it in clear text. + ## `.gittally.yml` Values shown are the defaults. @@ -48,8 +51,9 @@ server: publicBaseUrl: https://ci.example.org/ # HTTP port of the `server` subcommand (default 18080, like legacy) port: 18080 - # bind address of the `server` subcommand - bindAddress: 0.0.0.0 + # bind address of the `server` subcommand; loopback only, because the UI and the API + # are unauthenticated — set 0.0.0.0 only deliberately (see the note below) + bindAddress: 127.0.0.1 # optional Impressum (legal disclosure) link in the web UI footer; empty hides the link impressumUrl: "" # Opt-in managed nginx+certbot Docker container for HTTPS, for hosts without @@ -172,6 +176,13 @@ branches: - "04:00" ``` +### Notes on `server.bindAddress` + +The default is `127.0.0.1`. +Neither the web UI nor the JSON API authenticates read access, and every page carries the control token that unlocks the build controls, so GitTally is meant to sit behind the host's reverse proxy rather than on a public interface. +Set `0.0.0.0` only deliberately — for the managed nginx container (which reaches GitTally over the Docker bridge, not over loopback), or when the proxy runs on another host. +Installations created before v0.9.9 have `bindAddress: 0.0.0.0` written into their `.gittally.yml` and keep it; the new default only applies where the key is absent or `init` writes a fresh file. + ### Notes on `server.nginx` With `nginx.enabled`, the `server` subcommand also starts a managed nginx Docker container that serves GitTally over HTTPS (ADR 0005). diff --git a/docs/deployment.md b/docs/deployment.md index 4dafa10..e9bb411 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -101,7 +101,7 @@ All GitTally configuration lives in the YAML files described in [configuration.m ## Reverse Proxy (nginx) -Bind GitTally to localhost and set the public URL in `.gittally.yml`: +Bind GitTally to localhost — the default since v0.9.9 — and set the public URL in `.gittally.yml`: ```yaml server: @@ -197,5 +197,5 @@ All nginx and certificate failures are non-fatal warnings — the plain HTTP ser `serverName` must be a public DNS name pointing at the host, reachable from the internet on port 80/443 (directly or via a port forward to `httpPort`/`httpsPort`), otherwise the ACME challenge fails. The nginx container cannot reach `localhost` of the host, so the proxy upstream defaults to `serverName`; set `server.nginx.upstreamHost` if the host is reachable under a different name from inside containers. -With the managed nginx, keep `server.bindAddress: 0.0.0.0` (or an address reachable from the Docker network) — binding GitTally to `127.0.0.1` would make it unreachable for the proxy. +With the managed nginx, set `server.bindAddress: 0.0.0.0` explicitly (or an address reachable from the Docker network) — the default `127.0.0.1` makes GitTally unreachable for the proxy container. See [configuration.md](configuration.md) for all `server.nginx.*` keys. 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 093578e..2b7a9fb 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 @@ -65,7 +65,7 @@ Blast radius is limited to build-lifecycle operations (a DoS/integrity concern, #### TODO 3 — Accept the control token via header only -- [ ] Remove the `token` query-parameter variant from the three mutating endpoints in [`BuildsApiController.kt:90,115,129`](../../src/main/kotlin/de/hoennig/gittally/server/BuildsApiController.kt); keep only the `X-GitTally-Token` header (which the bundled UI already uses). +- [x] Remove the `token` query-parameter variant from the three mutating endpoints in [`BuildsApiController.kt:90,115,129`](../../src/main/kotlin/de/hoennig/gittally/server/BuildsApiController.kt); keep only the `X-GitTally-Token` header (which the bundled UI already uses). **Background.** Tokens in URLs are routinely written to access logs, reverse-proxy logs, browser history, and the `Referer` header on outbound navigation. @@ -74,8 +74,9 @@ The query-param path exists only for legacy convenience. #### TODO 4 — Redact the Gitea token in `config:print` -- [ ] Mask `git.token` (and any future secret) by default in [`ConfigPrintCommand.kt:20-31`](../../src/main/kotlin/de/hoennig/gittally/commands/ConfigPrintCommand.kt); gate the plaintext value behind an explicit `--show-secrets` flag. -- [ ] Update `tools/setup-gittally-instance:272`, which currently steers the operator to run `config:print --full` to view the token. +- [x] Mask `git.token` (and any future secret) by default in [`ConfigPrintCommand.kt:20-31`](../../src/main/kotlin/de/hoennig/gittally/commands/ConfigPrintCommand.kt); gate the plaintext value behind an explicit `--show-secrets` flag. + Masked as `***` on both the `--full` and the raw path, with a leading YAML comment naming the flag, so the output stays parseable when piped. +- [x] Update `tools/setup-gittally-instance:272`, which currently steers the operator to run `config:print --full` to view the token. **Background.** Both the `--full` and default branches print the token verbatim to stdout, landing it in terminal scrollback, `script(1)` captures, screen-shares, or CI logs. @@ -112,7 +113,8 @@ Before this PR all build config was loaded from the primary checkout via `config #### TODO 7 — Default `bindAddress` to `127.0.0.1` -- [ ] Change the default in [`GitTallyConfig.kt:21`](../../src/main/kotlin/de/hoennig/gittally/config/GitTallyConfig.kt) and the `init` template ([`InitCommand.kt:126`](../../src/main/kotlin/de/hoennig/gittally/commands/InitCommand.kt)) from `0.0.0.0` to `127.0.0.1`; require operators to opt into all-interfaces. +- [x] Change the default in [`GitTallyConfig.kt:21`](../../src/main/kotlin/de/hoennig/gittally/config/GitTallyConfig.kt) and the `init` template ([`InitCommand.kt:126`](../../src/main/kotlin/de/hoennig/gittally/commands/InitCommand.kt)) from `0.0.0.0` to `127.0.0.1`; require operators to opt into all-interfaces. + Shipped as v0.9.9 with the migration note in the release notes, `docs/configuration.md` ("Notes on `server.bindAddress`") and `docs/deployment.md`: existing configs keep their explicit value, and the managed nginx now needs `0.0.0.0` set deliberately. **Background.** The current default binds all interfaces, which — combined with the public read surface and token-in-HTML — exposes the whole UI and the control token to the network whenever the reverse proxy is forgotten. @@ -149,7 +151,7 @@ A small TOCTOU gap; `GitAskPass`'s atomic-at-creation approach is the pattern to - TODO 2: full fix (gating the pages) versus documentation-only — the pages are the UI, so gating them needs a decision on how operators authenticate; the current implemented behavior is fully public. - TODO 5: whether public read access is acceptable by design (it matches legacy) or should change; current behavior leaves all reads public. - TODO 6: the pinned set is settled (secrets + Gitea/server + `docker.enabled`/`docker.network`); the open point is whether `docker.env` should also be pinned, since a branch overriding it controls its own container's environment (currently proposed as worktree-overridable). -- TODO 7: changing the default `bindAddress` is a behavior change for existing installs that rely on `0.0.0.0`; needs a migration note. +- ~~TODO 7: changing the default `bindAddress` is a behavior change for existing installs that rely on `0.0.0.0`; needs a migration note.~~ Settled: the default changed in v0.9.9 and the migration note is in the release notes and both deployment docs. ## Additional Changes diff --git a/src/main/kotlin/de/hoennig/gittally/commands/ConfigPrintCommand.kt b/src/main/kotlin/de/hoennig/gittally/commands/ConfigPrintCommand.kt index da30278..9bba4c3 100644 --- a/src/main/kotlin/de/hoennig/gittally/commands/ConfigPrintCommand.kt +++ b/src/main/kotlin/de/hoennig/gittally/commands/ConfigPrintCommand.kt @@ -17,16 +17,41 @@ class ConfigPrintCommand( @Option(names = ["--full"], description = ["Include all defaults"]) var full: Boolean = false + @Option(names = ["--show-secrets"], description = ["Print secrets (git.token) in clear text instead of masked"]) + var showSecrets: Boolean = false + override fun run() { if (full) { - print(configLoader.toYaml(configLoader.load())) + val config = configLoader.load() + printMaskingNote(config.git.token) + print(configLoader.toYaml(if (showSecrets) config else config.copy(git = config.git.copy(token = MASK)))) } else { val raw = configLoader.loadRaw() if (raw.isEmpty()) { println("(no configuration files found)") } else { - print(configLoader.toYaml(raw)) + printMaskingNote(rawToken(raw)) + print(configLoader.toYaml(if (showSecrets) raw else maskToken(raw))) } } } + + /** A YAML comment, so the output stays parseable when piped into a file. */ + private fun printMaskingNote(token: String?) { + if (!showSecrets && !token.isNullOrEmpty()) { + println("# git.token is masked — pass --show-secrets to print it") + } + } + + private fun rawToken(raw: Map): String? = (raw["git"] as? Map<*, *>)?.get("token") as? String + + private fun maskToken(raw: Map): Map { + val git = raw["git"] as? Map<*, *> ?: return raw + if (rawToken(raw).isNullOrEmpty()) return raw + return raw + ("git" to git.entries.associate { (key, value) -> key to if (key == "token") MASK else value }) + } + + companion object { + private const val MASK = "***" + } } diff --git a/src/main/kotlin/de/hoennig/gittally/commands/InitCommand.kt b/src/main/kotlin/de/hoennig/gittally/commands/InitCommand.kt index 37c9135..a06f72a 100644 --- a/src/main/kotlin/de/hoennig/gittally/commands/InitCommand.kt +++ b/src/main/kotlin/de/hoennig/gittally/commands/InitCommand.kt @@ -125,8 +125,10 @@ class InitCommand( publicBaseUrl: "" # HTTP port of the `server` subcommand port: 18080 - # bind address of the `server` subcommand - bindAddress: 0.0.0.0 + # bind address of the `server` subcommand; loopback only, because the UI and the + # API are unauthenticated — use 0.0.0.0 only without a reverse proxy in front + # (and with the managed nginx below, which reaches GitTally from its container) + bindAddress: 127.0.0.1 # optional Impressum (legal disclosure) link in the web UI footer; empty hides the link impressumUrl: "" # Opt-in managed nginx+certbot Docker container for HTTPS, for hosts without diff --git a/src/main/kotlin/de/hoennig/gittally/config/GitTallyConfig.kt b/src/main/kotlin/de/hoennig/gittally/config/GitTallyConfig.kt index 6b230b2..61c969e 100644 --- a/src/main/kotlin/de/hoennig/gittally/config/GitTallyConfig.kt +++ b/src/main/kotlin/de/hoennig/gittally/config/GitTallyConfig.kt @@ -18,7 +18,12 @@ data class ServerConfig( val publicBaseUrl: String = "", /** HTTP port of the `server` subcommand; 18080 like the legacy artifact server. */ val port: Int = 18080, - val bindAddress: String = "0.0.0.0", + /** + * Loopback by default: the UI and the API are unauthenticated, so reaching them should + * require the host's reverse proxy. Set `0.0.0.0` explicitly to expose all interfaces — + * which the managed nginx container needs (see `docs/deployment.md`). + */ + val bindAddress: String = "127.0.0.1", /** Optional Impressum (legal disclosure) link shown in the web UI footer; empty hides the link. */ val impressumUrl: String = "", val nginx: NginxConfig = NginxConfig(), diff --git a/src/main/kotlin/de/hoennig/gittally/server/BuildsApiController.kt b/src/main/kotlin/de/hoennig/gittally/server/BuildsApiController.kt index 53ea946..400a85c 100644 --- a/src/main/kotlin/de/hoennig/gittally/server/BuildsApiController.kt +++ b/src/main/kotlin/de/hoennig/gittally/server/BuildsApiController.kt @@ -24,8 +24,10 @@ import java.nio.file.StandardOpenOption /** * JSON API over build results and running builds, replacing the legacy - * `/control/…` endpoints. Mutating endpoints are guarded by the control token - * (header [TOKEN_HEADER] or parameter `token`), like the legacy cancel token. + * `/control/…` endpoints. Mutating endpoints are guarded by the control token, + * like the legacy cancel token — in the header [TOKEN_HEADER] only: a token in + * the query string would end up in access logs, browser history and `Referer` + * headers, and it never expires. */ @RestController class BuildsApiController( @@ -91,9 +93,8 @@ class BuildsApiController( fun restart( @RequestParam branch: String, @RequestHeader(name = TOKEN_HEADER, required = false) headerToken: String?, - @RequestParam(name = "token", required = false) paramToken: String?, ): ResponseEntity { - rejectBadToken(headerToken ?: paramToken)?.let { return it } + rejectBadToken(headerToken)?.let { return it } val commit = repository.latestFor(branch)?.commit ?: gitService.originHeadCommit(branch, workingDir) @@ -116,9 +117,8 @@ class BuildsApiController( fun cancel( @PathVariable artifactKey: String, @RequestHeader(name = TOKEN_HEADER, required = false) headerToken: String?, - @RequestParam(name = "token", required = false) paramToken: String?, ): ResponseEntity { - rejectBadToken(headerToken ?: paramToken)?.let { return it } + rejectBadToken(headerToken)?.let { return it } if (!buildExecutor.cancel(artifactKey)) { return notFound("no queued or running build with artifact key '$artifactKey'") } @@ -130,9 +130,8 @@ class BuildsApiController( fun delete( @PathVariable artifactKey: String, @RequestHeader(name = TOKEN_HEADER, required = false) headerToken: String?, - @RequestParam(name = "token", required = false) paramToken: String?, ): ResponseEntity { - rejectBadToken(headerToken ?: paramToken)?.let { return it } + rejectBadToken(headerToken)?.let { return it } if (!repository.delete(artifactKey)) { return notFound("no build with artifact key '$artifactKey'") } diff --git a/src/main/resources/templates/releases.html b/src/main/resources/templates/releases.html index 8b29e88..2276c5b 100644 --- a/src/main/resources/templates/releases.html +++ b/src/main/resources/templates/releases.html @@ -7,6 +7,22 @@
+

v0.9.9 — 2026-08-11

+
    +
  • Changed default: server.bindAddress is now 127.0.0.1 + instead of 0.0.0.0, because neither the UI nor the API authenticates read access. + Existing .gittally.yml files keep whatever they set; with the managed nginx + container, 0.0.0.0 has to be set explicitly.
    • +
  • The control token is accepted in the X-GitTally-Token header only — the + token query parameter is gone, as URLs end up in access logs and browser history.
    • +
  • config:print masks git.token; --show-secrets prints it.
    • +
  • Files holding secrets (the Gitea token written by init, the control token) are + created with mode 0600 right away instead of being chmod-ed afterwards, + the control token is compared in constant time, and git calls pass -- before + branch names.
    • +
  • On narrow screens the page title and the repository name are stacked instead of wrapping.
    • +
+

v0.9.8 — 2026-08-10

  • The artifact index also links report pages of directories without an index.html. diff --git a/src/test/kotlin/de/hoennig/gittally/commands/ConfigPrintCommandTest.kt b/src/test/kotlin/de/hoennig/gittally/commands/ConfigPrintCommandTest.kt new file mode 100644 index 0000000..b1c9972 --- /dev/null +++ b/src/test/kotlin/de/hoennig/gittally/commands/ConfigPrintCommandTest.kt @@ -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() + private val command = ConfigPrintCommand(configLoader) + + private val rawConfig = + mapOf( + "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" + } + } +} diff --git a/src/test/kotlin/de/hoennig/gittally/server/BuildsApiControllerTest.kt b/src/test/kotlin/de/hoennig/gittally/server/BuildsApiControllerTest.kt index c8c996e..1c3b1b7 100644 --- a/src/test/kotlin/de/hoennig/gittally/server/BuildsApiControllerTest.kt +++ b/src/test/kotlin/de/hoennig/gittally/server/BuildsApiControllerTest.kt @@ -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()) } diff --git a/tools/setup-gittally-instance b/tools/setup-gittally-instance index ec88abf..184b6ef 100755 --- a/tools/setup-gittally-instance +++ b/tools/setup-gittally-instance @@ -275,7 +275,8 @@ Done. GitTally config for $hostname is in place. Verify and start on this host: cd $REPO_DIR - java -jar $JAR_PATH config:print --full # check the effective config + git.token + java -jar $JAR_PATH config:print --full # check the effective config (git.token masked) + java -jar $JAR_PATH config:print --full --show-secrets # ... including git.token in clear text java -jar $JAR_PATH server # or: java -jar $JAR_PATH init --systemd Note: GitTally binds to $hostname:${GITTALLY_ARTIFACT_SERVER_PORT:-18080} over plain HTTP.