feat(build): add parameterized install target (lr-3b7a)#12
Conversation
Add make install with PREFIX/DESTDIR overrides so any operator (and NAOMI post_merge) can install the built binary with no environment- specific paths hardcoded. Defaults to /usr/local/bin, matching FHS convention; overridable via PREFIX= and/or DESTDIR=. Document usage in README. Build artifact was already gitignored.
![clagentic-reviewer[bot]](https://avatars.githubusercontent.com/in/3944489?s=60&v=4){kind=small}
There was a problem hiding this comment.
PEACHES — clean
Findings
None. The parameterized install target is correctly designed:
install -d $(BINDIR)creates directories portably (POSIX-standard).- PREFIX/DESTDIR/BINDIR fully parameterized; no hardcoded paths (rule 11, amos.code-craft.10).
- Build artifact cleanup correct;
/gatekeeperalready gitignored. - README Install section accurately documents the behavior and override examples.
- GOPATH/GOCACHE philosophy preserved (no forced env vars in recipes).
Note for operator
Inspection-only review (no sandbox make install execution). Recipe is sound for typical system paths. If BINDIR contains spaces or special chars, unquoted expansion could split — but this is an edge case outside the stated use case (standard /usr/local and staging roots).
![clagentic-security[bot]](https://avatars.githubusercontent.com/in/4134208?s=60&v=4){kind=small}
There was a problem hiding this comment.
BOBBIE — clean
Scope confirmed: only Makefile and README.md changed. No Go source, no .crew/, no dependency manifests touched.
Reviewed the parameterized install target for injection / path-expansion surface (PREFIX, DESTDIR, BINDIR):
PREFIX ?= /usr/local,DESTDIR ?=,BINDIR ?= $(DESTDIR)$(PREFIX)/binare Make variables set only by the invoker ofmake install(operator or CI job), not by any PR-supplied or remotely-controlled input in this diff. No trust boundary is crossed — the party who could set an adversarial PREFIX/DESTDIR already has shell-equivalent control over the invocation.- Unquoted
$(BINDIR)ininstall -d $(BINDIR)andinstall -m 0755 gatekeeper $(BINDIR)/gatekeeperis real: a PREFIX/DESTDIR containing a shell metacharacter (e.g.;) would be spliced unquoted into a shell command line executed by make's/bin/shrecipe shell (Makefile lines 25–26). This is the same pattern used by nearly all upstream Makefiles (autotools, coreutils) and does not enable privilege escalation across an actor boundary in this PR — downgraded from PEACHES' "robustness nit" framing to a security-flavored nit rather than dropped, since quoting closes both the whitespace-splitting and the metacharacter-injection failure mode. Recommend"$(BINDIR)"quoting on both install lines. rm -f gatekeeper(Makefile line 27) removes a fixed relative filename in CWD — no traversal risk.- No secrets, tokens, hostnames, PII, or machine-specific absolute paths introduced (gitleaks: no leaks found; semgrep --config=auto: 0 findings).
- README Install section documents PREFIX/DESTDIR overrides accurately; no exposure.
Findings:
- Makefile:25-26 — bobbie.sast.2 — nit: PREFIX/DESTDIR/BINDIR expand unquoted into
install -d/install -mshell recipe lines; a value containing shell metacharacters would be word-split/interpreted by sh. No privilege boundary crossed in this PR (invoker-controlled var, same actor executes make), so not blocking; quote"$(BINDIR)"to close the case where PREFIX is inherited from a less-trusted CI env var.
scanners_run: gitleaks (no leaks found), semgrep --config=auto (0 findings, 47 rules, 1 file), osv-scanner (not applicable — no dependency manifest changed in this diff)
{"reviewer": "bobbie", "review_status": "clean", "head_sha": "dbe4abadeb126d4cc41485390d66a861ebb6039e", "pr_number": 12}
Closes both whitespace-splitting and shell-metacharacter interpretation of $(DESTDIR)$(PREFIX)/bin paths in the install target — flagged by PEACHES and BOBBIE as a non-blocking nit on defense-in-depth grounds for a released Makefile.
There was a problem hiding this comment.
PEACHES — clean (0 findings)
Summary
Re-review at amended head 0292e03: quoting nit is resolved. The install target now properly quotes $(BINDIR) expansions in both install -d and install invocations, preventing word-splitting if PREFIX or DESTDIR contain spaces. No other changes vs. prior clean review; still only Makefile + README. Execution confirmed by operator on-host.
Checked
- amos.code-craft.1 (minimal change) — only Makefile and README.md modified
- amos.code-craft.6 (no bare except) — N/A for Makefile
- amos.code-craft.8 (no TODO without task id) — N/A
- amos.code-craft.9 (no new deps) — N/A
- amos.code-craft.10 (no hardcoded secrets/hosts/paths) — all paths parameterized; no environment-specific strings
- CLAUDE.md rule 11 (build to share, no hardcoded machine names) — PREFIX, DESTDIR, BINDIR fully parameterized; examples in README are doc-only
- Quoting safety —
"$(BINDIR)"and"$(BINDIR)/gatekeeper"properly quoted
No blocking findings.
There was a problem hiding this comment.
PEACHES — clean (0 findings)
Quoting nit resolved. The install recipe now properly quotes "$(BINDIR)" expansions (lines 34–35, Makefile). Operator execution confirmed clean at head 0292e03: make install PREFIX=/tmp/gk-test installed a runnable binary to /tmp/gk-test/bin/gatekeeper with no repo artifacts. No hardcoded paths (rule CLAUDE.md #11). Changes scoped to Makefile + README only. All parameterized install variables (PREFIX, DESTDIR, BINDIR) are fully documented. Regression-free vs prior clean review.
There was a problem hiding this comment.
BOBBIE — clean
Re-audit at amended head 0292e03 (prior clean audit was at dbe4aba, now superseded).
Change since prior audit: the install recipe now quotes both $(BINDIR) expansions:
install -d "$(BINDIR)"(Makefile:34)install -m 0755 gatekeeper "$(BINDIR)/gatekeeper"(Makefile:35)
This resolves the prior nit (bobbie.sast.2 — unquoted make variable expansion vulnerable to word-splitting/glob-expansion if PREFIX/DESTDIR contain spaces or shell metacharacters). Confirmed by direct read of Makefile at current head: both BINDIR expansions are double-quoted.
Scope confirmed unchanged: diff touches only Makefile and README.md (install target + docs). No new files, no dependency changes, no secrets.
Scanners:
- gitleaks detect (working tree, no-git): 0 leaks
- semgrep --config auto (Makefile, README.md): 0 findings across 47 rules
No new exposure introduced. No hardcoded paths (PREFIX/DESTDIR remain fully overridable, defaulting to /usr/local per workspace build-to-share rule). Operator confirmed clean execution via make install PREFIX=/tmp/gk-test.
Findings: none.
scanners_run: gitleaks(ok, 0 findings), semgrep(ok, 0 findings), osv-scanner(not applicable — no dependency changes in diff)
review.status: clean
There was a problem hiding this comment.
BOBBIE — clean
Re-audit at amended head 0292e03 (prior clean audit was at dbe4aba, now superseded).
Change since prior audit: the install recipe now quotes both $(BINDIR) expansions:
install -d "$(BINDIR)"(Makefile:34)install -m 0755 gatekeeper "$(BINDIR)/gatekeeper"(Makefile:35)
This resolves the prior nit (bobbie.sast.2 — unquoted make variable expansion vulnerable to word-splitting/glob-expansion if PREFIX/DESTDIR contain spaces or shell metacharacters). Confirmed by direct read of Makefile at current head: both BINDIR expansions are double-quoted.
Scope confirmed unchanged: diff touches only Makefile and README.md (install target + docs). No new files, no dependency changes, no secrets.
Scanners:
- gitleaks detect (working tree, no-git): 0 leaks
- semgrep --config auto (Makefile): 0 findings across 47 rules
No new exposure introduced. No hardcoded paths (PREFIX/DESTDIR remain fully overridable, defaulting to /usr/local). Operator confirmed clean execution via make install PREFIX=/tmp/gk-test.
Findings: none.
scanners_run: gitleaks(ok, 0 findings), semgrep(ok, 0 findings), osv-scanner(not applicable — no dependency changes in diff)
review.status: clean
audit_scope.head_sha: 0292e03
clagentic gate-note — authorized
Authorize rationale: PEACHES clean + BOBBIE clean at 0292e03; live head verified via ls-remote refs/pull/12/head; Makefile/README-only change; authorized by holden + andy 2026-07-04 (lr-3b7a) |
clagentic gate-note — authorized
Authorize rationale: PEACHES clean + BOBBIE clean at 0292e03 (re-review/audit after quoting amendment); operator-confirmed make install execution; authorized by holden + andy 2026-07-04 (lr-3b7a) |
Amend to PR #12 addressing reviewer nit and adding real execution verification for lr-3b7a. What changed: Quoted BINDIR path expansions in the Makefile install recipe (the install -d and install -m 0755 lines) to close whitespace-splitting and shell-metacharacter interpretation, per defense-in-depth guidance for a released Makefile. Why: PEACHES and BOBBIE both flagged the unquoted BINDIR expansion as a non-blocking nit on PR 12; fixing before merge. Execution verification: go test bare (no env prefix) failed with module cache not found, neither GOMODCACHE nor GOPATH is set; the AMoS guard-bash allowlist rejects any VAR=val prefix or env wrapper in front of go, so the required inline env vars GOPATH GOMODCACHE GOCACHE could not be supplied. make install with PREFIX override is blocked outright since the allowlist permits only make build test lint vet fmt check verbs; install is excluded regardless of env vars. Binary landing verification and cleanup were unreachable since install never ran. This is a genuine allowlist blocker, not a claimed pass; no fabricated execution results. Task lr-3b7a. tests_passed is null for this amendment: blocked by the Bash allowlist, not run, not green, not claimed; the diff itself is a one-line quoting fix with no behavioral change to test.