Skip to content

fix: harden Dockerfile templates — eliminate silent failures, use APT best practices#594

Open
jerome-benoit wants to merge 1 commit into
mattpocock:mainfrom
jerome-benoit:fix/harden-dockerfile-templates
Open

fix: harden Dockerfile templates — eliminate silent failures, use APT best practices#594
jerome-benoit wants to merge 1 commit into
mattpocock:mainfrom
jerome-benoit:fix/harden-dockerfile-templates

Conversation

@jerome-benoit
Copy link
Copy Markdown
Contributor

Summary

Harden all Dockerfile templates to eliminate silent download failures and follow APT best practices.

Problem

The current GITHUB_CLI_TOOLS constant and all agent Dockerfile templates use pipe patterns that silently swallow failures:

# Current — if curl fails, dd exits 0 with empty file → cryptic GPG error later
RUN curl -fsSL https://...keyring.gpg \
  | dd of=/usr/share/keyrings/githubcli-archive-keyring.gpg

Solution

# Fixed — curl failure stops the build immediately
RUN install -m 0755 -d /etc/apt/keyrings \
  && curl -fsSL https://...keyring.gpg \
     -o /etc/apt/keyrings/githubcli-archive-keyring.gpg \
  && chmod a+r /etc/apt/keyrings/githubcli-archive-keyring.gpg

Changes

  • src/InitService.ts: Hardened GITHUB_CLI_TOOLS constant (no pipes, /etc/apt/keyrings/, --no-install-recommends). Added --no-install-recommends to base packages in all 4 agent Dockerfile templates + BEADS_TOOLS.
  • .sandcastle/Dockerfile: Same hardening applied to the repo's dogfood Dockerfile.
  • .changeset/harden-dockerfile-templates.md: Patch changeset.

What this fixes for users

  • sandcastle init now generates Dockerfiles that fail fast on network errors instead of building silently broken images
  • Smaller images via --no-install-recommends
  • Keyrings in the modern /etc/apt/keyrings/ location per Debian policy

Closes #593

Copilot AI review requested due to automatic review settings May 7, 2026 10:05
@vercel
Copy link
Copy Markdown

vercel Bot commented May 7, 2026

@jerome-benoit is attempting to deploy a commit to the Matt Pocock's projects Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the generated Dockerfile templates (and the repo’s own .sandcastle/Dockerfile) to avoid silent failures during GitHub CLI APT key/repo setup and to follow Debian/Docker APT best practices.

Changes:

  • Reworked GitHub CLI installation steps to avoid curl | dd / echo | tee, use /etc/apt/keyrings, and install with --no-install-recommends.
  • Added --no-install-recommends to base package installs across the agent Dockerfile templates and Beads tools.
  • Added a patch changeset documenting the behavior change.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

File Description
src/InitService.ts Updates Dockerfile template strings (APT install flags + hardened GitHub CLI setup) and adjusts some template text formatting.
.sandcastle/Dockerfile Applies the same hardened GitHub CLI APT keyring/repo setup and --no-install-recommends to the dogfood image.
.changeset/harden-dockerfile-templates.md Adds a patch changeset describing the Dockerfile hardening.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/InitService.ts Outdated
ARG AGENT_GID=1000

# Rename the base image's "node" user to "agent" and align UID/GID.
# Rename the base image's \"node\" user to \"agent\" and align UID/GID.
Comment thread src/InitService.ts Outdated
ARG AGENT_GID=1000

# Rename the base image's "node" user to "agent" and align UID/GID.
# Rename the base image's \"node\" user to \"agent\" and align UID/GID.
Comment thread src/InitService.ts Outdated
ARG AGENT_GID=1000

# Rename the base image's "node" user to "agent" and align UID/GID.
# Rename the base image's \"node\" user to \"agent\" and align UID/GID.
Comment thread src/InitService.ts Outdated
ARG AGENT_GID=1000

# Rename the base image's "node" user to "agent" and align UID/GID.
# Rename the base image's \"node\" user to \"agent\" and align UID/GID.
Comment thread src/InitService.ts
Comment on lines +247 to 255
const GITHUB_CLI_TOOLS = `# GitHub CLI
RUN install -m 0755 -d /etc/apt/keyrings \\
&& curl -fsSL https://cli.github.com/packages/githubcli-archive-keyring.gpg \\
-o /etc/apt/keyrings/githubcli-archive-keyring.gpg \\
&& chmod a+r /etc/apt/keyrings/githubcli-archive-keyring.gpg \\
&& echo "deb [arch=$(dpkg --print-architecture) signed-by=/etc/apt/keyrings/githubcli-archive-keyring.gpg] https://cli.github.com/packages stable main" \\
> /etc/apt/sources.list.d/github-cli.list \\
&& apt-get update && apt-get install -y --no-install-recommends gh \\
&& rm -rf /var/lib/apt/lists/*`;
Comment thread .sandcastle/Dockerfile
Comment on lines +10 to +17
# GitHub CLI
RUN install -m 0755 -d /etc/apt/keyrings \
&& curl -fsSL https://cli.github.com/packages/githubcli-archive-keyring.gpg \
-o /etc/apt/keyrings/githubcli-archive-keyring.gpg \
&& chmod a+r /etc/apt/keyrings/githubcli-archive-keyring.gpg \
&& echo "deb [arch=$(dpkg --print-architecture) signed-by=/etc/apt/keyrings/githubcli-archive-keyring.gpg] https://cli.github.com/packages stable main" \
> /etc/apt/sources.list.d/github-cli.list \
&& apt-get update && apt-get install -y --no-install-recommends gh \
@jerome-benoit
Copy link
Copy Markdown
Contributor Author

Regarding the curl | bash comments (#5, #6):

These are intentionally kept. The distinction:

  • curl | dd (fixed) — dd always exits 0 even with empty input → silent broken keyring → cryptic GPG error later
  • curl -fsSL ... | bash (kept) — vendor-recommended install mechanism for Claude Code and Beads. With -f, curl exits non-zero on HTTP errors. Rewriting to download-then-execute would require knowing the install script's expected filename/location, and diverges from vendor docs.

If we want belt-and-suspenders here, SHELL ["/bin/bash", "-o", "pipefail", "-c"] at the top of the Dockerfile would catch pipe failures globally — but that's a separate concern from APT key management hardening, and changes the default shell for all subsequent RUN instructions. Happy to add it as a follow-up if desired.

… best practices

- Replace curl|dd and echo|tee pipes with curl -o and > redirects
- Migrate APT keyrings from /usr/share/keyrings/ to /etc/apt/keyrings/
- Add --no-install-recommends to all apt-get install calls
- Apply same hardening to .sandcastle/Dockerfile (dogfood)

Closes mattpocock#593
@jerome-benoit jerome-benoit force-pushed the fix/harden-dockerfile-templates branch from 27e4639 to b918ba1 Compare May 7, 2026 10:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Harden Dockerfile templates: eliminate silent failures, use APT best practices

2 participants