Skip to content

fix(phase4): repair candidate artifact layout and guard Linux acceptance #75

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
gitcommit90 merged 1 commit into from
Aug 4, 2026
Merged

fix(phase4): repair candidate artifact layout and guard Linux acceptance #75

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions .github/workflows/candidate.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,6 @@ jobs:
dist/1Helm-*-linux-split.json
dist/artifact-size-report.json
dist/artifact-size-report.txt
container/channel-machine.oci.json
dist/candidate-evidence/candidate.json
dist/candidate-evidence/archive.sha256
dist/candidate-evidence/manifest.sha256
Expand All @@ -205,7 +204,6 @@ jobs:
path: |
dist/1Helm-channel-machine-v1-*.oci.tar
dist/1Helm-channel-machine-v1-*.json
container/channel-machine.oci.sha256
if-no-files-found: error
retention-days: 90

Expand Down Expand Up @@ -308,6 +306,9 @@ jobs:
actions: read
attestations: read
steps:
- name: Clear retained runner workspace from prior candidates
run: rm -rf -- candidate-download candidate-result

- name: Download this workflow's exact candidate
uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4
with:
Expand Down
17 changes: 17 additions & 0 deletions ops/platform-acceptance/linux.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,23 @@ export HELM_ACCEPTANCE_STARTED_AT="$STARTED_AT"
export HELM_PHASE4_RUNNER_LABEL=ubuntu-latest
node "$ROOT/scripts/pending-acceptance-evidence.mjs"
[[ "$(id -u)" -ne 0 ]] || { echo "Linux acceptance must begin as the hosted ordinary runner user." >&2; exit 1; }

# This lane performs a REAL root install of 1Helm on its runner: it binds port
# 8123, writes /var/lib/1helm-oci-v1, and installs the 1helm systemd units. That
# is only safe on a disposable GitHub-hosted runner that holds no user or
# production data. Refuse anywhere that looks persistent, self-hosted, or
# already-inhabited so a misrouted job can never clobber a real host's live
# 1Helm or standalone state. Blocked evidence was already retained above.
[[ "${RUNNER_ENVIRONMENT:-}" == "github-hosted" ]] \
|| { echo "Linux acceptance refuses to boot a real 1Helm outside a disposable GitHub-hosted runner." >&2; exit 1; }
for guarded in /var/lib/1helm-oci-v1 /var/lib/1helm-standalone /opt/1helm; do
[[ ! -e "$guarded" ]] \
|| { echo "Linux acceptance refuses to run where 1Helm host state already exists: $guarded" >&2; exit 1; }
done
if command -v ss >/dev/null 2>&1 && ss -ltn 2>/dev/null | grep -qE '[:.]8123[[:space:]]'; then
echo "Linux acceptance refuses to run while port 8123 is already in use." >&2
exit 1
fi
Comment on lines +40 to +43

@coderabbitai coderabbitai Bot Aug 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Fail closed when port inspection is unavailable.

If ss is absent or returns an error, the if condition is false and the script continues to Line 64, where it performs the root installation. The runner can therefore proceed while port 8123 is occupied. Require ss and handle inspection errors before running grep.

Proposed fix 
-if command -v ss >/dev/null 2>&1 && ss -ltn 2>/dev/null | grep -qE '[:.]8123[[:space:]]'; then
+command -v ss >/dev/null 2>&1 \
+  || { echo "Linux acceptance cannot verify port 8123 because ss is unavailable." >&2; exit 1; }
+ports="$(ss -ltn 2>/dev/null)" \
+  || { echo "Linux acceptance cannot verify port 8123." >&2; exit 1; }
+if grep -qE '[:.]8123[[:space:]]' <<<"$ports"; then
   echo "Linux acceptance refuses to run while port 8123 is already in use." >&2
   exit 1
 fi

As per PR objectives, Linux acceptance must verify that port 8123 is available before root installation.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if command -v ss >/dev/null 2>&1 && ss -ltn 2>/dev/null | grep -qE '[:.]8123[[:space:]]'; then
echo "Linux acceptance refuses to run while port 8123 is already in use." >&2
exit 1
fi
command -v ss >/dev/null 2>&1 \
|| { echo "Linux acceptance cannot verify port 8123 because ss is unavailable." >&2; exit 1; }
ports="$(ss -ltn 2>/dev/null)" \
|| { echo "Linux acceptance cannot verify port 8123." >&2; exit 1; }
if grep -qE '[:.]8123[[:space:]]' <<<"$ports"; then
echo "Linux acceptance refuses to run while port 8123 is already in use." >&2
exit 1
fi
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ops/platform-acceptance/linux.sh` around lines 40 - 43, Update the port
validation before the root installation in the Linux acceptance script to fail
closed when ss is unavailable or its inspection command fails. Require ss
explicitly, run ss port inspection separately, handle any nonzero status by
printing an error and exiting, and only then use grep to reject an occupied port
8123.

[[ "$(sha256sum "$ARCHIVE" | awk '{print $1}')" == "$DIGEST" ]] \
|| { echo "Linux candidate digest mismatch." >&2; exit 1; }
[[ "$(sha256sum "$OFFLINE_ARCHIVE" | awk '{print $1}')" == "$OFFLINE_DIGEST" ]] \
Expand Down
5 changes: 5 additions & 0 deletions test/phase2-candidate.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,17 +115,22 @@ test("rollback fixtures remain local-only and cannot satisfy normal candidate va

test("candidate workflow and guest boundary exclude PR code and broad root access", () => {
const workflow = read(".github/workflows/candidate.yml");
const candidateUpload = workflow.match(/- name: Upload exact candidate and evidence[\s\S]*?retention-days: 30/)?.[0] || "";
const helper = read("ops/dress-rehearsal/1helm-candidate-install");
const hook = read("ops/dress-rehearsal/runner-job-started.sh");
const sudoersExample = "%actions ALL=(root) NOPASSWD: /usr/local/sbin/1helm-candidate-install \"\"\n";
assert.match(workflow, /workflow_run:[\s\S]*workflows: \[CI\][\s\S]*branches: \[main\]/);
assert.match(workflow, /workflow_run\.event == 'push'/);
assert.match(workflow, /head_repository\.full_name == github\.repository/);
assert.match(workflow, /runs-on: \[1helm-dress-rehearsal-phase2\]/);
assert.match(workflow, /Clear retained runner workspace[\s\S]*rm -rf -- candidate-download candidate-result[\s\S]*Download this workflow's exact candidate/);
assert.match(workflow, /github\.sha == github\.event\.workflow_run\.head_sha/);
assert.match(workflow, /attest-build-provenance@[a-f0-9]{40}/);
assert.match(workflow, /candidate-download\/candidate-evidence\/candidate\.json/);
assert.match(workflow, /candidate-download\/candidate-evidence\/provenance\.bundle\.json/);
assert.match(candidateUpload, /dist\/1Helm-\*-linux-node\.tgz/);
assert.match(candidateUpload, /dist\/candidate-evidence\/candidate\.json/);
assert.doesNotMatch(candidateUpload, /container\/channel-machine\.oci\.json/);
assert.match(helper, /--signer-workflow gitcommit90\/1Helm\/\.github\/workflows\/candidate\.yml/);
assert.match(helper, /--source-ref refs\/heads\/main/);
assert.match(helper, /--source-digest "\$commit"/);
Expand Down
10 changes: 10 additions & 0 deletions test/phase4-platform-acceptance.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,16 @@ test("workflow routes no PR/fork code, uses unique labels, fans acceptance out,
assert.doesNotMatch(workflow, /pull_request_target|workflow_dispatch/);
assert.doesNotMatch(workflow.match(/assemble-promotion:[\s\S]*?(?=\n candidate-status:)/)?.[0] || "", /npm (ci|install|run build|run package)/);
assert.match(workflow, /Upload exact Linux acceptance evidence\n if: always\(\)/);
// Both retained artifacts must be single-rooted under dist/ so consumers find
// files at the download root, not nested under dist/ (the layout bug that
// broke Linux, Windows, and Phase 2 candidate discovery in one run).
const channelUpload = workflow.match(/- name: Retain immutable digest-addressed channel image candidate[\s\S]*?retention-days: 90/)?.[0] || "";
assert.doesNotMatch(channelUpload, /container\//);
assert.match(channelUpload, /dist\/1Helm-channel-machine-v1-\*\.oci\.tar/);
Comment on lines +118 to +120

@coderabbitai coderabbitai Bot Aug 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Assert the complete channel-image metadata contract.

The test checks only the OCI tar path. The workflow at .github/workflows/candidate.yml Lines 205-206 retains dist/1Helm-channel-machine-v1-*.json and removes the .oci.sha256 sidecar. Assert both conditions so a metadata regression cannot pass this test.

Proposed assertions 
   assert.doesNotMatch(channelUpload, /container\//);
   assert.match(channelUpload, /dist\/1Helm-channel-machine-v1-\*\.oci\.tar/);
+  assert.match(channelUpload, /dist\/1Helm-channel-machine-v1-\*\.json/);
+  assert.doesNotMatch(channelUpload, /channel-machine\.oci\.sha256/);

As per PR objectives, the channel-image artifact must retain its intended single-root layout and metadata.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const channelUpload = workflow.match(/- name: Retain immutable digest-addressed channel image candidate[\s\S]*?retention-days: 90/)?.[0] || "";
assert.doesNotMatch(channelUpload, /container\//);
assert.match(channelUpload, /dist\/1Helm-channel-machine-v1-\*\.oci\.tar/);
const channelUpload = workflow.match(/- name: Retain immutable digest-addressed channel image candidate[\s\S]*?retention-days: 90/)?.[0] || "";
assert.doesNotMatch(channelUpload, /container\//);
assert.match(channelUpload, /dist\/1Helm-channel-machine-v1-\*\.oci\.tar/);
assert.match(channelUpload, /dist\/1Helm-channel-machine-v1-\*\.json/);
assert.doesNotMatch(channelUpload, /channel-machine\.oci\.sha256/);
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/phase4-platform-acceptance.mjs` around lines 118 - 120, Extend the
channelUpload assertions in the phase4 acceptance test to verify the complete
metadata contract: match the retained dist/1Helm-channel-machine-v1-*.json
metadata file and assert that the .oci.sha256 sidecar is absent, while
preserving the existing single-root and OCI tar path checks.

const linuxAccept = read("ops/platform-acceptance/linux.sh");
assert.match(linuxAccept, /RUNNER_ENVIRONMENT.*==.*"github-hosted"/);
assert.match(linuxAccept, /1helm-standalone/);
assert.match(linuxAccept, /refuses to run while port 8123 is already in use/);
Comment on lines +121 to +124

@coderabbitai coderabbitai Bot Aug 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Assert every guarded host-state path.

The test checks only 1helm-standalone. It does not require /var/lib/1helm-oci-v1 or /opt/1helm from ops/platform-acceptance/linux.sh Lines 36-39. A regression could remove either guard while this test still passes.

Proposed assertions 
   assert.match(linuxAccept, /RUNNER_ENVIRONMENT.*==.*"github-hosted"/);
-  assert.match(linuxAccept, /1helm-standalone/);
+  assert.match(linuxAccept, /\/var\/lib\/1helm-oci-v1/);
+  assert.match(linuxAccept, /\/var\/lib\/1helm-standalone/);
+  assert.match(linuxAccept, /\/opt\/1helm/);
   assert.match(linuxAccept, /refuses to run while port 8123 is already in use/);

As per PR objectives, Linux acceptance must reject existing 1Helm and standalone host state.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const linuxAccept = read("ops/platform-acceptance/linux.sh");
assert.match(linuxAccept, /RUNNER_ENVIRONMENT.*==.*"github-hosted"/);
assert.match(linuxAccept, /1helm-standalone/);
assert.match(linuxAccept, /refuses to run while port 8123 is already in use/);
const linuxAccept = read("ops/platform-acceptance/linux.sh");
assert.match(linuxAccept, /RUNNER_ENVIRONMENT.*==.*"github-hosted"/);
assert.match(linuxAccept, /\/var\/lib\/1helm-oci-v1/);
assert.match(linuxAccept, /\/var\/lib\/1helm-standalone/);
assert.match(linuxAccept, /\/opt\/1helm/);
assert.match(linuxAccept, /refuses to run while port 8123 is already in use/);
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/phase4-platform-acceptance.mjs` around lines 121 - 124, Update the Linux
acceptance assertions in the phase4 platform test to require the guarded
host-state paths /var/lib/1helm-oci-v1 and /opt/1helm, in addition to the
existing 1helm-standalone check. Keep the current RUNNER_ENVIRONMENT and
port-8123 assertions unchanged so the test covers every host-state rejection
path in the Linux acceptance script.

assert.match(workflow.match(/accept-macos:[\s\S]*?(?=\n accept-windows:)/)?.[0] || "", /if: always\(\)[\s\S]*name: 1helm-macos-acceptance-/);
assert.match(workflow.match(/accept-windows:[\s\S]*?(?=\n assemble-promotion:)/)?.[0] || "", /if: always\(\)[\s\S]*name: 1helm-windows-acceptance-/);
});
Expand Down
Loading