Skip to content

Gate the deploy before validation, and match the bundle's directory mode - #21

Merged
ptr727 merged 1 commit into
developfrom
fix/deploy-review-round-two
Aug 4, 2026
Merged

Gate the deploy before validation, and match the bundle's directory mode#21
ptr727 merged 1 commit into
developfrom
fix/deploy-review-round-two

Conversation

@ptr727

@ptr727 ptr727 commented Aug 4, 2026

Copy link
Copy Markdown
Owner

Three findings from the promotion review, all valid.

Job ordering contradicted its own comment

assert-ref was documented as running before Hugo is installed, and ran beside validate, which installs Hugo and builds the site. A production deploy dispatched from the wrong ref therefore paid for a full validation before refusing. The gate now runs first and validate needs it.

The upload cleared setgid

make-release.sh assembles with --chmod=D2755 deliberately, so the group propagates below the first directory rsync creates. The upload used D755, which strips that bit on the remote tree and makes the on-host permission model diverge from the one the bundle was built and tested against. Confirmed on a real assembled release:

drwxr-sr-x  .../current/site

The upload now uses --no-g --chmod=D2755,F644, matching assembly.

A relative environment file could traverse

The resolution claimed to anchor a relative ENV_FILE under the repo and did not, so ../other.env reached outside it. A relative name exists to reach secrets/, so traversal is refused rather than resolved. An absolute path is still honoured, which is how a file outside the repo is named deliberately.

../other.env                 -> ENV_FILE must not traverse
secrets/../../escape.env     -> ENV_FILE must not traverse
/nonexistent/abs.env         -> environment file not found
secrets/staging.env          -> resolves under the repo

Verification

Both mirrors pass 1245 URLs honored. actionlint, shellcheck, and shfmt clean.

🤖 Generated with Claude Code

The ref assertion ran beside validation rather than before it, so a
mis-dispatched production deploy installed a toolchain and built a site
before refusing.

The upload cleared the setgid bit the bundle is assembled with, so the
group would stop propagating below the first directory the transfer
creates.

A relative environment file is refused rather than resolved when it
traverses, since a relative name is meant to reach the secrets directory.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 4, 2026 22:51

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This pull request tightens the deploy pipeline by enforcing an early ref gate, aligns remote upload permissions with the release bundle’s intended directory mode, and hardens environment-file selection to prevent relative-path traversal.

Changes:

  • Reorders the deploy workflow so assert-ref runs first and validate depends on it, preventing unnecessary installs/builds on mis-dispatched production deploys.
  • Updates the deployment rsync upload flags to preserve the intended setgid directory mode (D2755) and avoid group preservation (--no-g), matching bundle assembly expectations.
  • Rejects relative ENV_FILE values that include traversal (..) while still allowing absolute paths.

Reviewed changes

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

File Description
deploy/make-release.sh Refuses traversal in relative ENV_FILE values before anchoring under the repo.
.github/workflows/deploy-site.yml Runs assert-ref first and makes validate depend on it; deploy depends on validate.
.github/workflows/deploy-site-task.yml Aligns rsync upload permissions (--no-g --chmod=D2755,F644) with the bundle’s directory-mode contract.

@ptr727
ptr727 merged commit 6b2c739 into develop Aug 4, 2026
5 checks passed
@ptr727
ptr727 deleted the fix/deploy-review-round-two branch August 4, 2026 22:54
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.

2 participants