Skip to content

.github: ci.yml: move AWS auth to just before it's needed - #3018

Merged
ES-Alexander merged 1 commit into
bluerobotics:masterfrom
ES-Alexander:ci-reduce-aws-auth-risk
Sep 4, 2026
Merged

.github: ci.yml: move AWS auth to just before it's needed#3018
ES-Alexander merged 1 commit into
bluerobotics:masterfrom
ES-Alexander:ci-reduce-aws-auth-risk

Conversation

@ES-Alexander

@ES-Alexander ES-Alexander commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Applies an improvement spurred by the #3017 (backport) review.

Avoids an AWS auth failure stopping the rest of the deployment targets from receiving updated images, and avoids unrelated steps from having access to an authenticated AWS user.

This makes the electron deployment better match what was already being done for the flatpak deployment (defined just underneath it).

Avoids an AWS auth failure stopping the rest of the deployment targets from receiving updated images, and avoids unrelated steps from having access to an authenticated AWS user
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
READY TO MERGE (Automated PR Review — round 1)

0 open findings.

When Cockpit cuts a stable release, the release job logs into Amazon's file storage so it can drop a copy of the installer at a fixed "latest stable" address. That login used to happen near the start of the job, before the installers were even built, so if the login failed nothing else got published either — no installer on the GitHub release page, no auto-update file. This moves the login to the last possible moment, right before the one command that needs it. Everything else in the release now finishes first, and the rest of the job no longer runs while holding live cloud credentials.

What still needs attention

Nothing is open — no findings were raised this round.

Change map — what was established before judging

Claims

The PR body asserts two effects; both check out.

  1. "Avoids an AWS auth failure stopping the rest of the deployment targets from receiving updated images"verified, with a wording caveat. The step moves from .github/workflows/ci.yml:311-317 to sit immediately before Upload AWS Build (of Stable Release) at ci.yml:395-399. In the old position it preceded every Deploy step (ci.yml:319-341) and every release upload (ci.yml:358-393), so a failure there aborted the job before any of them ran: that matrix leg produced no installer, no GitHub Release asset, and no latest*.yml auto-update metadata. The caveat is on "deployment targets": the sibling jobs deploy-flatpak (ci.yml:401) and deploy-blueos-extension (ci.yml:497) were never at risk — they are separate jobs — and the other matrix legs were protected by fail-fast: false (ci.yml:185). What was actually at risk was the rest of this leg's own outputs, the auto-update metadata most of all.
  2. "avoids unrelated steps from having access to an authenticated AWS user"verified. The nine steps between the old and new positions previously ran with AWS_ACCESS_KEY_ID / AWS_SECRET_ACCESS_KEY exported into their environment, including the five yarn ${{ matrix.deployCommand }} invocations (ci.yml:319-341) that run electron-builder and the project's own build scripts. After the move only Upload AWS Build sees them.

Also worth stating plainly, since neither the body nor the diff says it: the step is gated on startsWith(github.ref, 'refs/tags/v') && !contains(github.ref, '-') (ci.yml:313), so this whole path is inert on pull requests, branch pushes, the 6-day cron, and prerelease tags. The change only takes effect on a stable release run.

Failure site — not a bug fix in the product sense, but the misbehaving arrangement is in the diff: the auth step's old position at ci.yml:311-317 is exactly what the diff removes. There is no second site — deploy-flatpak already has the step in the correct late position (ci.yml:483-489, immediately before its own Upload AWS Build at ci.yml:491-495), so this PR brings deploy-electron into line with the pattern already in the tree rather than inventing one.

Entry points

Function Reached from Frequency
Configure AWS Credentials (for Stable Release) (step, deploy-electron) push of a refs/tags/v* tag with no -, via the deploy-electron job (needs: test), across all 5 matrix legs one-shot (per stable release, per matrix leg)
Upload AWS Build (of Stable Release) (step, unchanged, now the immediate successor) same one-shot (per stable release, per matrix leg)

Nothing else in the job changed. No step was added, removed, renamed, or re-conditioned — the 8 added lines are byte-identical to the 8 removed ones.

Invariants

The move relies on two rules:

  1. The credentials exported by aws-actions/configure-aws-credentials are still in the environment when aws s3 cp runs. The action exports them as job-scoped environment variables, so every later step in the same job sees them; the two steps are now adjacent (ci.yml:384-399 post-move), which is the strongest form of this. Covered.
  2. No step earlier in deploy-electron needs AWS credentials. Enumerated exhaustively: a case-insensitive tree-wide search for aws / AWS_ / s3:// (excluding node_modules) returns the credential block and the aws s3 cp line in each of the two deploy jobs and nothing else — the src/ hits are substring matches inside words like "draws". The indirect route is closed too: every deploy:* script in package.json:14-20 passes --publish=never, and electron-builder's configured provider is github (package.json:193-196), so the Deploy steps never reach S3. Covered.
  3. The two if: conditions cannot diverge. The auth step's guard (ci.yml:313) is character-identical to the upload step's (ci.yml:396), so "auth skipped but upload ran" is unreachable. Unchanged by this PR, and the adjacency makes future divergence more visible rather than less.

Failure paths after the change (this file is under .github/, so it was read in full rather than as hunks):

  • AWS auth fails → the job fails at that step. The GitHub Release already has the installer, the mac diff zip, and the latest*.yml auto-update metadata; only the version-less S3 copy is missing, so the static "latest stable" URL keeps serving the previous release rather than 404ing. The run is red, which is the correct signal.
  • aws s3 cp fails → still fails the job. No || true, no continue-on-error, no swallowed exit code was introduced anywhere in the diff, so a broken stable upload cannot pass silently.
  • Non-stable ref → both steps skip, exactly as before.
Sections with nothing to report (11)

1. Correctness & Implementation Bugs — ✅ (traced every consumer of the credentials: a tree-wide aws/s3:// search finds only ci.yml:399 in this job, and package.json:14-20 passes --publish=never with provider github at package.json:193-196, so no step between the old and new position used them; the moved if: at ci.yml:313 is character-identical to the upload's at ci.yml:396, and aws s3 cp still carries no || true or continue-on-error)

2. Persistence & User Data — ✅ (no cockpit-* key, no settings-management.ts or useBlueOsStorage call site touched; the diff is 8 relocated YAML lines inside one CI job)

3. AGENTS.md Adherence — ✅ (8 lines removed and the same 8 re-added with no edit; no rename, reorder, reflow, new dependency, or unused groundwork, and the diff matches the stated purpose exactly)

4. Security — ✅ (the same two secrets.AWS_* references move unchanged and now reach strictly fewer steps — the five yarn ${{ matrix.deployCommand }} invocations at ci.yml:319-341 no longer run with live credentials; aws-actions/configure-aws-credentials@v6 is unpinned-by-tag as before with no version change, and the 34-line diff carries no encoded blob, hidden unicode, or new network endpoint)

5. Performance — ✅ (the relocated step is one-shot per stable release per matrix leg, per the entry-point walk; no runtime code, no bundle, and no wall-clock change to any job)

6. UI / UX — ✅ (no .vue, template, or user-facing string in the diff; the only user-visible artifact, the static s3://bcloud-public/cockpit/releases/stable/ URL, is written by the same unchanged command)

7. Code Quality & Style — ✅ (complexity-report.json reports changedFiles: 0, functionsMeasured: 0, truncated: false and no triggered entries, consistent with a YAML-only diff; nothing was measurable and nothing fired)

8. Commit Hygiene — ✅ (one commit for one logical change; git log on this file shows 35 ci: and one workflows:, so .github: ci.yml: is a novel prefix form, but it names the area it changes, which is what the AGENTS.md rule asks; no issue or PR reference in the message — the #3017 mention is in the PR body where it belongs — and no wip/fixup!/AI-authorship trailer)

9. Tests — ✅ (no test touched or weakened; the test job's nine self-check steps at ci.yml:119-141 and the complexity-report job at ci.yml:58-95 are untouched)

10. Documentation — ✅ (nothing in the Lite/Standalone README table changes; on a successful run the published artifacts and their URLs are byte-for-byte what they were)

11. Nitpicks / Optional — ✅ (read the moved block against the deploy-flatpak precedent at ci.yml:483-495; the placement, indentation, and step name now match it exactly)

Generated by Claude. This is advisory; a human reviewer must still approve.

@ES-Alexander
ES-Alexander merged commit c84f8a8 into bluerobotics:master Sep 4, 2026
18 checks passed
@ES-Alexander
ES-Alexander deleted the ci-reduce-aws-auth-risk branch September 4, 2026 18:37
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