sneak/secret
1
0
Fork 0
Code Issues 1 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

Makefile hardcodes a personal remote DOCKER_HOST, breaking make docker for everyone else #44

New Issue
Closed
opened 2026-08-09 03:42:32 +02:00 by clawbot · 1 comment
clawbot commented 2026-08-09 03:42:32 +02:00
Collaborator
Copy Link
Copy Source

From the repo-standards audit. Present on main and the pending lint branch.

Problem

Makefile:2:

export DOCKER_HOST := ssh://root@ber1app1.local

:= assigns unconditionally, overriding any inherited DOCKER_HOST. Consequences:

  • make docker silently targets one specific machine on a private LAN, over SSH as root, for every user of this repo.
  • Any contributor, and any CI runner that invokes the make target rather than script/cibuild, fails with an unresolvable host and an error that gives no hint why.
  • Nobody but the author can build the image locally at all.

Policy: "Simple projects should be configured with environment variables", and the Makefile is meant to be "authoritative documentation for how the repo is used". A hardcoded personal host is neither.

Two smaller Makefile defects while in the same file:

Missing .PHONY entries. The list covers default bootstrap setup test lint fmt fmt-check check docker hooks vet but omits build, clean, install, and docker-run. build is the live risk: the Dockerfile runs make build at line 31, and a file or directory named build in the repo root would make the target a silent no-op, producing an image with a stale or missing binary.

Targets that bypass script/. The nine policy-named targets are correct thin shims, but build runs go build inline, and vet, clean, install, docker-run invoke tooling directly. build matters most because the Dockerfile depends on it, so real build logic lives in the Makefile rather than in script/ where policy puts it. vet is also redundant — script/test already runs go vet first.

Definition of done

  • DOCKER_HOST is no longer unconditionally assigned. Either remove the line entirely, or make it opt-in and overridable with ?= and an empty default.
  • make docker works on a stock local Docker installation with no environment setup.
  • .PHONY includes build, clean, install, and docker-run.
  • script/build exists as a POSIX sh entrypoint following the conventions of the other scripts (#!/bin/sh, set -eu, the repo-root cd idiom), and build: becomes a thin shim to it.
  • The Dockerfile still builds successfully with the reworked build target.
  • The redundant vet target is removed, or kept with a one-line comment saying why it earns its place.
  • make check green. TODO.md updated in the same commit.

Implementation requirements

  • Do not relocate the ldflags version-injection logic in a way that breaks it. Whatever moves into script/build must still produce a binary with the version string populated — build it and run the version command to confirm, since nothing in the test suite covers this.
  • Coordinate with #43 (module path rename), which also edits the ldflags lines. Whichever lands second must carry the other's change forward.
  • If removing the DOCKER_HOST line would disrupt your own workflow, the ?= form plus a line in the README noting it can be set in the environment is the better option — but the default must be a working local build.
From the repo-standards audit. Present on `main` and the pending lint branch. ## Problem `Makefile:2`: ```make export DOCKER_HOST := ssh://root@ber1app1.local ``` `:=` assigns unconditionally, overriding any inherited `DOCKER_HOST`. Consequences: - `make docker` silently targets one specific machine on a private LAN, over SSH as root, for every user of this repo. - Any contributor, and any CI runner that invokes the make target rather than `script/cibuild`, fails with an unresolvable host and an error that gives no hint why. - Nobody but the author can build the image locally at all. Policy: *"Simple projects should be configured with environment variables"*, and the Makefile is meant to be *"authoritative documentation for how the repo is used"*. A hardcoded personal host is neither. Two smaller Makefile defects while in the same file: **Missing `.PHONY` entries.** The list covers `default bootstrap setup test lint fmt fmt-check check docker hooks vet` but omits `build`, `clean`, `install`, and `docker-run`. `build` is the live risk: the `Dockerfile` runs `make build` at line 31, and a file or directory named `build` in the repo root would make the target a silent no-op, producing an image with a stale or missing binary. **Targets that bypass `script/`.** The nine policy-named targets are correct thin shims, but `build` runs `go build` inline, and `vet`, `clean`, `install`, `docker-run` invoke tooling directly. `build` matters most because the Dockerfile depends on it, so real build logic lives in the Makefile rather than in `script/` where policy puts it. `vet` is also redundant — `script/test` already runs `go vet` first. ## Definition of done - `DOCKER_HOST` is no longer unconditionally assigned. Either remove the line entirely, or make it opt-in and overridable with `?=` and an empty default. - `make docker` works on a stock local Docker installation with no environment setup. - `.PHONY` includes `build`, `clean`, `install`, and `docker-run`. - `script/build` exists as a POSIX sh entrypoint following the conventions of the other scripts (`#!/bin/sh`, `set -eu`, the repo-root `cd` idiom), and `build:` becomes a thin shim to it. - The `Dockerfile` still builds successfully with the reworked `build` target. - The redundant `vet` target is removed, or kept with a one-line comment saying why it earns its place. - `make check` green. `TODO.md` updated in the same commit. ## Implementation requirements - Do not relocate the ldflags version-injection logic in a way that breaks it. Whatever moves into `script/build` must still produce a binary with the version string populated — build it and run the version command to confirm, since nothing in the test suite covers this. - Coordinate with #43 (module path rename), which also edits the ldflags lines. Whichever lands second must carry the other's change forward. - If removing the `DOCKER_HOST` line would disrupt your own workflow, the `?=` form plus a line in the README noting it can be set in the environment is the better option — but the default must be a working local build.
clawbot added this to the 1.0.0 milestone 2026-08-09 03:42:32 +02:00
clawbot commented 2026-10-04 07:33:16 +02:00
Author
Collaborator
Copy Link
Copy Source

Built in #84: the Makefile no longer sets DOCKER_HOST, so its docker targets use the local daemon, or a DOCKER_HOST set in the environment. make build now calls the new script/build, which stamps the version and commit as before; a VERSION exported in the shell now counts too. build, clean, install and docker-run are in .PHONY, and the vet target is removed because script/test already runs go vet.

Model: opus-5-5

Built in https://git.eeqj.de/sneak/secret/pulls/84: the `Makefile` no longer sets `DOCKER_HOST`, so its docker targets use the local daemon, or a `DOCKER_HOST` set in the environment. `make build` now calls the new `script/build`, which stamps the version and commit as before; a `VERSION` exported in the shell now counts too. `build`, `clean`, `install` and `docker-run` are in `.PHONY`, and the `vet` target is removed because `script/test` already runs `go vet`. Model: opus-5-5
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
critical
Extremely important: data loss, a security hole, production down, or basic use impossible
 merge-ready
needs-checks
needs-rebase
needs-review
needs-rework
No labels
Milestone
No items
No Milestone 1.0.0
Projects
Clear projects
No projects
Assignees
Clear assignees
clawbot sneak (Jeffrey Paul)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/secret#44