ci: port release shell scripts to Node ESM#46
Conversation
Replace the seven `.github/scripts` shell scripts with nine `actions/github-script` ESM modules (`.mjs`), invoked from the workflows via dynamic `import()`. Each module is `@ts-check`'d with JSDoc types against `@actions/github-script` and receives the injected `core`/`github`/`context`/`exec`, so the scripts carry no runtime dependencies. Adds `.node-version`, the `@actions/github-script` / `@types/node` / `typescript` devDeps, and the supporting `package.json` / `tsconfig.json` / `bun.lock` / `.gitignore` updates. Built on the `workflow_dispatch` release design (release.yml dispatches npm/aur publishes and the publishers poll for the build run). This predates master's `workflow_call` chaining; reconciling the two designs is tracked as integration work.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches✨ Simplify code
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/npm-release.yml (1)
45-50:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winArrr! Ye be missin' the
.node-versionfile from yer sparse-checkout, matey!The
setup-nodestep referencesnode-version-file: .node-version, but the sparse-checkout patterns don't include this file. This'll cause the step to either fail or fall back to some unexpected Node version, sinkin' yer entire publish voyage!🏴☠️ Proposed fix to add the missing file
with: ref: ${{ github.event.repository.default_branch }} persist-credentials: false sparse-checkout: | .github/scripts npm/targets.json + .node-version sparse-checkout-cone-mode: false🤖 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 @.github/workflows/npm-release.yml around lines 45 - 50, The sparse-checkout block is missing the .node-version file referenced by the setup-node step (node-version-file: .node-version), so update the sparse-checkout patterns under sparse-checkout to include ".node-version" (alongside .github/scripts and npm/targets.json) so the setup-node step can read the file and use the intended Node version; locate the sparse-checkout section in the workflow and add the .node-version entry.
🤖 Prompt for all review comments with 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.
Inline comments:
In @.github/scripts/build/man-archive.mjs:
- Around line 20-24: Validate and sanitize environment inputs before using them
to build filenames: check env.RELEASE_TAG (releaseTag) with a strict semver
validator (e.g. semver.valid(...) or an equivalent regex) and call fail(core,
"...") if invalid, and for env.TARGET ensure it contains no path separators (no
'/' or '\\') before using it; apply the same validation pattern to
package-release-asset.mjs as well so both scripts reject traversal characters
and only accept safe, validated values.
- Line 29: The tar invocation using exec.exec("tar", ["-C", "man", "-czf",
archive, "."]) runs without ensuring the man/ directory exists; add a defensive
check (e.g., fs.existsSync or fs.promises.stat) for the "man" directory before
calling exec.exec and call fail(...) with a clear error message if it is missing
so tar doesn't produce a cryptic error. Locate the call to exec.exec in
.github/scripts/build/man-archive.mjs and gate it behind the existence check
referencing the same archive variable and preserve existing error handling flow.
In @.github/scripts/build/package-release-asset.mjs:
- Around line 68-73: The catch is swallowing all copyFile errors and always
reporting "not found"; update the try/catch around the await copyFile(src,
join(staging, bin)) to capture the thrown error (e.g., `err`), check err.code
and if err.code === 'ENOENT' call fail(core, `${src} not found — build step did
not produce ${bin}`), otherwise call fail(core, `${src} copy failed:
${err.message}`) (or include err) so permission, disk, and other I/O errors are
reported accurately; keep the existing return behavior after failing.
In @.github/scripts/build/verify-checksum.mjs:
- Line 68: The current code builds the SHA256 digest by reading the entire file
into memory via readFile; change this to stream-based hashing: create a read
stream from join(dir, expected) (using createReadStream), pipe it into
createHash('sha256') and await the stream completion (using stream.pipeline or
stream/promises.pipeline), then call hash.digest('hex') to produce digest;
update imports to include createReadStream and pipeline if needed and replace
the existing await readFile(...) usage that assigns to digest.
In @.github/scripts/release/notes.mjs:
- Around line 26-31: The current assembly of body using release.data.body and
generated.data.body can duplicate generated notes when the job reruns; update
the logic that builds body (the body constant) to first detect and remove any
previously appended generated.data.body from release.data.body (or check if
release.data.body already includes generated.data.body) before joining, so that
joining release.data.body and generated.data.body is idempotent and does not
produce duplicated content.
In @.github/workflows/crates-release.yml:
- Around line 11-12: The workflow-level permissions block currently grants
id-token: write to every job (permissions: { contents: read, id-token: write });
move the id-token: write permission out of the top-level permissions and add it
only to the job that runs the crates-io-auth-action / publishes to crates.io
(the job that performs the release/publish step), keeping contents: read at
workflow level if needed or scoping it to jobs that require it; update the job's
permissions block to include id-token: write and remove id-token from the global
permissions to enforce least privilege.
In @.github/workflows/release.yml:
- Around line 228-243: The workflow step that dynamically imports
publishGithubRelease and dispatchPackagePublishes from "${{ github.workspace
}}/.github/scripts/release/publish.mjs" can fail because the repository files
are not checked out; add a checkout step (e.g., actions/checkout) before the
actions/github-script steps so that the workspace contains the
.github/scripts/release/publish.mjs file and the imports succeed; ensure the
checkout step appears prior to the steps invoking publishGithubRelease and
dispatchPackagePublishes.
In @.gitignore:
- Around line 17-19: The current unignore line "!.cargo" exposes the entire
.cargo tree (risking token/credential commits); remove or stop unignoring the
whole directory and instead explicitly unignore only safe artifacts while adding
explicit ignore patterns for credentials. Update the .gitignore by removing or
replacing the "!.cargo" entry and add explicit ignores such as
".cargo/credentials", ".cargo/credentials.toml", ".cargo/*.token",
".cargo/*.key", and any other credential filename patterns your project uses; if
you need to unignore specific safe files, unignore those exact filenames or
patterns rather than the entire .cargo directory.
In `@package.json`:
- Line 13: The dependency pin for "`@actions/github-script`" in package.json is
referencing a non-existent commit SHA; update the dependency spec for
"`@actions/github-script`" to a valid reference (either a published npm version
like a release tag e.g. "github-script@^6.1.0" or a real commit/annotated tag
from the actions/github-script repo) so installations and audits succeed; locate
the "`@actions/github-script`" entry in package.json and replace the fake
"github:actions/github-script#373c709c69115d41ff229c7e5df9f8788daa9553" value
with a valid tag/commit or semver release.
---
Outside diff comments:
In @.github/workflows/npm-release.yml:
- Around line 45-50: The sparse-checkout block is missing the .node-version file
referenced by the setup-node step (node-version-file: .node-version), so update
the sparse-checkout patterns under sparse-checkout to include ".node-version"
(alongside .github/scripts and npm/targets.json) so the setup-node step can read
the file and use the intended Node version; locate the sparse-checkout section
in the workflow and add the .node-version entry.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 68be6ee7-911f-4b57-bb34-9671114ada59
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (24)
.github/scripts/build/build-npm-packages.sh.github/scripts/build/derive-dist-dry.sh.github/scripts/build/download-release-archives.mjs.github/scripts/build/download-release-archives.sh.github/scripts/build/man-archive.mjs.github/scripts/build/npm.mjs.github/scripts/build/package-release-asset.mjs.github/scripts/build/package-release-asset.sh.github/scripts/build/verify-checksum.mjs.github/scripts/build/verify-checksum.sh.github/scripts/publish/aur-prepare.sh.github/scripts/publish/aur.mjs.github/scripts/publish/npm.mjs.github/scripts/publish/npm.sh.github/scripts/release/notes.mjs.github/scripts/release/publish.mjs.github/workflows/aur-release.yml.github/workflows/crates-release.yml.github/workflows/npm-release.yml.github/workflows/release.yml.gitignore.node-versionpackage.jsontsconfig.json
💤 Files with no reviewable changes (7)
- .github/scripts/publish/aur-prepare.sh
- .github/scripts/build/download-release-archives.sh
- .github/scripts/build/derive-dist-dry.sh
- .github/scripts/build/package-release-asset.sh
- .github/scripts/build/build-npm-packages.sh
- .github/scripts/build/verify-checksum.sh
- .github/scripts/publish/npm.sh
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
@(package.json|pyproject.toml|setup.py|Cargo.toml|go.mod|pom.xml|build.gradle|VERSION)
📄 CodeRabbit inference engine (Custom checks)
@(package.json|pyproject.toml|setup.py|Cargo.toml|go.mod|pom.xml|build.gradle|VERSION): If any source code files (excluding tests, docs, CI, markdown, or comments-only changes) are modified, then a version field MUST be updated in one of the following files if present in the repo: package.json, pyproject.toml, setup.py, Cargo.toml, go.mod, pom.xml, build.gradle, or a VERSION file.
The new version MUST follow SemVer (MAJOR.MINOR.PATCH). If the PR introduces breaking changes (removal or renaming of public APIs, changes to function signatures, deleted exported symbols, or incompatible config changes), MAJOR must increment. If it adds backward-compatible functionality, MINOR must increment. If it only fixes bugs without changing public APIs, PATCH must increment.
Files:
package.json
.github/**/*.{yml,yaml}
⚙️ CodeRabbit configuration file
Do not ever warn about stylistic yamllint shit. Do warn about security related shit. Insecure shell handling of user supplied / defined strings: think of branch names, inputs, pr content, anything needs to be string interpolation and permission safe.
Files:
.github/workflows/crates-release.yml.github/workflows/aur-release.yml.github/workflows/npm-release.yml.github/workflows/release.yml
🧠 Learnings (5)
📚 Learning: 2026-03-26T20:08:23.153Z
Learnt from: kjanat
Repo: kjanat/runner PR: 1
File: src/detect.rs:22-22
Timestamp: 2026-03-26T20:08:23.153Z
Learning: In `src/detect.rs` within the `kjanat/runner` codebase, `detect()` is intentionally infallible and should return `ProjectContext` directly. Parsing/I/O failures from task extractors (which return `anyhow::Result<Vec<String>>`) must be recorded by `push_extracted_tasks` into `ProjectContext.warnings` as `DetectionWarning` entries and then emitted to stderr via `cmd::print_warnings`. When reviewing, do not flag `detect()` for “swallowing errors” as long as failures are routed into the warnings-list design (i.e., they’re preserved in `ProjectContext.warnings`, not dropped silently).
Applied to files:
.node-version.gitignore
📚 Learning: 2026-05-04T22:42:28.879Z
Learnt from: kjanat
Repo: kjanat/runner PR: 5
File: CHANGELOG.md:8-27
Timestamp: 2026-05-04T22:42:28.879Z
Learning: In `CHANGELOG.md`, do not treat a `## [Unreleased]` section as an error/mismatch during development PRs even if `Cargo.toml` includes an in-progress version bump. The versioned header format `## [X.Y.Z] - YYYY-MM-DD` should only be added at release/tag time; this repo’s `CHANGELOG.md` includes a “Post-release checklist” confirming that behavior. Only flag changelog/version mismatches if they contradict the intended workflow described in `CHANGELOG.md`.
Applied to files:
.node-version.gitignore
📚 Learning: 2026-05-14T15:04:39.496Z
Learnt from: kjanat
Repo: kjanat/runner PR: 27
File: src/cli.rs:780-788
Timestamp: 2026-05-14T15:04:39.496Z
Learning: In `src/cli.rs`, do not recommend removing `trailing_var_arg = true` or `allow_hyphen_values = true` from `Command::Run.args` and `RunAliasCli.args`. They are intentionally required for the documented pass-through contract: hyphen-prefixed args (e.g., `--watch`) that appear after a task name must be forwarded to the underlying task. As a result, in chain mode, chain-failure flags like `-k`/`--kill-on-fail` that are placed after task names will be silently consumed as task args rather than parsed as flags; when proposing fixes or usage changes, use the documented workaround of placing all chain flags before task names (e.g., `runner run -s -k build test`). This behavior is verified in `src/lib.rs` (`run_alias_forwards_trailing_args`) and described in the README, so keep this design trade-off intact.
Applied to files:
.node-version.gitignore
📚 Learning: 2026-05-02T17:50:01.271Z
Learnt from: kjanat
Repo: kjanat/runner PR: 5
File: .gitignore:4-4
Timestamp: 2026-05-02T17:50:01.271Z
Learning: In this repository, `package-lock.json` is intentionally added to `.gitignore` and should not be committed. In future reviews, do not flag the absence/presence of `package-lock.json` as an issue as long as `.gitignore` continues to exclude it intentionally.
Applied to files:
.gitignore
📚 Learning: 2026-05-04T15:23:38.296Z
Learnt from: kjanat
Repo: kjanat/runner PR: 5
File: .github/workflows/release.yml:73-73
Timestamp: 2026-05-04T15:23:38.296Z
Learning: For GitHub Actions workflows in this repository (kjanat/runner), maintain the maintainer’s preference for “floating” action tags (e.g., `v6`, `v7`) instead of pinned full commit SHAs. During code review, do not flag action usages in `.github/workflows/**/*.yml` for not being pinned to full commit SHAs or suggest switching to SHA pinning.
Applied to files:
.github/workflows/crates-release.yml.github/workflows/aur-release.yml.github/workflows/npm-release.yml.github/workflows/release.yml
🪛 zizmor (1.25.2)
.github/workflows/crates-release.yml
[error] 11-11: overly broad permissions (excessive-permissions): id-token: write is overly broad at the workflow level
(excessive-permissions)
[warning] 11-11: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment
(undocumented-permissions)
.github/workflows/aur-release.yml
[warning] 33-33: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
.github/workflows/npm-release.yml
[warning] 36-36: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment
(undocumented-permissions)
[warning] 60-60: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 72-72: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 83-83: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
.github/workflows/release.yml
[warning] 42-42: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 62-62: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 160-160: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 183-183: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 194-194: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 201-201: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 234-234: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 242-242: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 227-227: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment
(undocumented-permissions)
🔇 Additional comments (29)
.github/scripts/publish/npm.mjs (7)
1-21: LGTM!
22-47: LGTM!
49-88: LGTM!
90-190: LGTM!
192-301: LGTM!
303-387: LGTM!
389-443: LGTM!.github/workflows/npm-release.yml (2)
1-28: LGTM!
52-84: LGTM!.github/workflows/crates-release.yml (1)
22-22: LGTM!.github/workflows/aur-release.yml (4)
28-34: Ahoy, the static analysis tool be raisin' false alarms about template injection, but this be safe waters!Zizmor flagged
${{ github.workspace }}as potentially attacker-controllable, but it be a runner-provided context value pointing to the checkout directory — not user input. The checkout step (line 26) pulls fromdefault_branch, so the imported script comes from trusted code. This pattern matches the other workflows in yer fleet (e.g.,release.ymlline 38).No action needed, Captain. The warning can be safely ignored.
Source: Linters/SAST tools
1-12: LGTM!
14-26: LGTM!
36-48: LGTM!.github/scripts/publish/aur.mjs (8)
1-13: LGTM!
15-43: LGTM!
45-51: LGTM!
53-96: LGTM!
98-103: LGTM!
105-128: LGTM!
130-141: LGTM!
143-166: LGTM!.github/scripts/release/publish.mjs (1)
35-53: Dispatch orchestration isn’t theworkflow_callthing you fear: both.github/workflows/npm-release.ymland.github/workflows/aur-release.ymlstill declareon: workflow_dispatch. Arrr—so this won’t hard-fail just because it’screateWorkflowDispatch.But ye’re passing
"run-id": String(context.runId)fornpm-release.yml; double-check thatnpm-release.ymlactually defines arun-idinput under itsworkflow_dispatch—otherwise the dispatch can still be rejected..node-version (1)
1-1: LGTM!.github/scripts/build/package-release-asset.mjs (1)
59-89: Shiver me timbers! Ye forgot to set permissions on the copied files before tarballing! ⚓Wait... no, I be seein' it now at line 74. Ye DO call
chmod(..., 0o755)on each binary after copyin'. That be shipshape! Carry on, sailor! 🏴☠️✨package.json (1)
18-19: Avast! Node 26.3.0 and npm 11.16.0 actually exist—now check security + “Current” vs LTS"node": "26.3.0", "npm": "11.16.0"Ye’re not bumpin’ into imaginary versions: Node.js 26.3.0 and npm 11.16.0 are real releases. Also, Node 26.3.0 is “Current”, not LTS—if ye need LTS stability, don’t sail with “Current”.
Still run/confirm vulnerability impact for these exact engine versions (e.g., via npm audit / advisory checks) before merging.tsconfig.json (1)
26-26: LGTM!.github/scripts/build/download-release-archives.mjs (1)
24-54: LGTM!Also applies to: 60-63
.github/scripts/build/npm.mjs (1)
24-24: 🏴☠️ Direct.tsexecution is the intended way to runbuild-packages.tsunder the pinned Node
npm/scripts/build-packages.tsexplicitly documents being run asnode npm/scripts/build-packages.ts ..., and.github/scripts/build/npm.mjsdoes exactly that (exec.exec("node", ["npm/scripts/build-packages.ts", ...])). With.node-versionpinned tov26.3.0, the real failure risk is only if the pinned Node/runtime behaviour changes (e.g., native TS type-stripping disabled).
| const releaseTag = env.RELEASE_TAG; | ||
| if (!releaseTag) { | ||
| fail(core, "RELEASE_TAG env var is required"); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Arrr! Both scripts be vulnerable to the same path traversal scurvy! 🏴☠️
Both man-archive.mjs and package-release-asset.mjs accept RELEASE_TAG (and package-release-asset.mjs also accepts TARGET) from environment variables and immediately use them in filename construction without validation. A malicious tag like ../../etc/passwd or v1.0/../evil could write files outside the intended directory. Apply consistent semver validation to RELEASE_TAG in both scripts, and validate that TARGET contains no path separators.
🤖 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 @.github/scripts/build/man-archive.mjs around lines 20 - 24, Validate and
sanitize environment inputs before using them to build filenames: check
env.RELEASE_TAG (releaseTag) with a strict semver validator (e.g.
semver.valid(...) or an equivalent regex) and call fail(core, "...") if invalid,
and for env.TARGET ensure it contains no path separators (no '/' or '\\') before
using it; apply the same validation pattern to package-release-asset.mjs as well
so both scripts reject traversal characters and only accept safe, validated
values.
| const archive = `runner-${releaseTag}-man.tar.gz`; | ||
| const checksum = `runner-${releaseTag}-man.sha256`; | ||
|
|
||
| await exec.exec("tar", ["-C", "man", "-czf", archive, "."]); |
There was a problem hiding this comment.
Arrr, this tar command be lookin' mighty suspicious! 🏴☠️
Ye be usin' tar -C man -czf with a hardcoded "." as the source, but there be no check that the man/ directory actually exists before running tar! If the directory be missing, tar will fail with a cryptic error instead of yer nice fail() message.
🗺️ Defensive check for man/ directory
+ import { access } from "node:fs/promises";
+ import { constants } from "node:fs";
+
const archive = `runner-${releaseTag}-man.tar.gz`;
const checksum = `runner-${releaseTag}-man.sha256`;
+ try {
+ await access("man", constants.R_OK);
+ } catch {
+ fail(core, "man/ directory not found — man pages must be built first");
+ return;
+ }
+
await exec.exec("tar", ["-C", "man", "-czf", archive, "."]);🤖 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 @.github/scripts/build/man-archive.mjs at line 29, The tar invocation using
exec.exec("tar", ["-C", "man", "-czf", archive, "."]) runs without ensuring the
man/ directory exists; add a defensive check (e.g., fs.existsSync or
fs.promises.stat) for the "man" directory before calling exec.exec and call
fail(...) with a clear error message if it is missing so tar doesn't produce a
cryptic error. Locate the call to exec.exec in
.github/scripts/build/man-archive.mjs and gate it behind the existence check
referencing the same archive variable and preserve existing error handling flow.
| try { | ||
| await copyFile(src, join(staging, bin)); | ||
| } catch { | ||
| fail(core, `${src} not found — build step did not produce ${bin}`); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Nya~ The error handling here is too vague, onii-chan! 💢
The empty catch block swallows ALL errors from copyFile() — not just ENOENT (file not found), but also EACCES (permission denied), ENOSPC (disk full), and other I/O errors. The generic "not found" message will be confusing if the real problem is permissions or disk space.
🎀 More specific error handling
for (const bin of ["runner", "run"]) {
const src = join(binDir, bin);
try {
await copyFile(src, join(staging, bin));
- } catch {
- fail(core, `${src} not found — build step did not produce ${bin}`);
+ } catch (err) {
+ const reason = err.code === 'ENOENT'
+ ? 'not found — build step did not produce'
+ : `failed to copy: ${err.message}`;
+ fail(core, `${src} ${reason} ${bin}`);
return;
}📝 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.
| try { | |
| await copyFile(src, join(staging, bin)); | |
| } catch { | |
| fail(core, `${src} not found — build step did not produce ${bin}`); | |
| return; | |
| } | |
| try { | |
| await copyFile(src, join(staging, bin)); | |
| } catch (err) { | |
| const reason = err.code === 'ENOENT' | |
| ? 'not found — build step did not produce' | |
| : `failed to copy: ${err.message}`; | |
| fail(core, `${src} ${reason} ${bin}`); | |
| return; | |
| } |
🤖 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 @.github/scripts/build/package-release-asset.mjs around lines 68 - 73, The
catch is swallowing all copyFile errors and always reporting "not found"; update
the try/catch around the await copyFile(src, join(staging, bin)) to capture the
thrown error (e.g., `err`), check err.code and if err.code === 'ENOENT' call
fail(core, `${src} not found — build step did not produce ${bin}`), otherwise
call fail(core, `${src} copy failed: ${err.message}`) (or include err) so
permission, disk, and other I/O errors are reported accurately; keep the
existing return behavior after failing.
| return; | ||
| } | ||
|
|
||
| const digest = createHash("sha256").update(await readFile(join(dir, expected))).digest("hex"); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Hash tarballs via stream, not full-buffer reads.
Line 68 loads each archive fully into memory before hashing. Streaming gives the same result with far steadier memory usage on larger assets.
Suggested refactor
import { createHash } from "node:crypto";
+import { createReadStream } from "node:fs";
import { readdir, readFile } from "node:fs/promises";
import { join } from "node:path";
import { cwd, env } from "node:process";
@@
- const digest = createHash("sha256").update(await readFile(join(dir, expected))).digest("hex");
+ const digest = await sha256File(join(dir, expected));
if (digest !== parsed.hex) {
fail(core, `checksum mismatch for ${expected}: expected ${parsed.hex}, got ${digest}`);
return;
}
}
@@
}
+
+/**
+ * `@param` {string} filePath
+ * `@returns` {Promise<string>}
+ */
+async function sha256File(filePath) {
+ const hash = createHash("sha256");
+ await new Promise((resolve, reject) => {
+ const stream = createReadStream(filePath);
+ stream.on("error", reject);
+ stream.on("data", (chunk) => hash.update(chunk));
+ stream.on("end", resolve);
+ });
+ return hash.digest("hex");
+}🤖 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 @.github/scripts/build/verify-checksum.mjs at line 68, The current code
builds the SHA256 digest by reading the entire file into memory via readFile;
change this to stream-based hashing: create a read stream from join(dir,
expected) (using createReadStream), pipe it into createHash('sha256') and await
the stream completion (using stream.pipeline or stream/promises.pipeline), then
call hash.digest('hex') to produce digest; update imports to include
createReadStream and pipeline if needed and replace the existing await
readFile(...) usage that assigns to digest.
| const body = `${ | ||
| [release.data.body, generated.data.body] | ||
| .map((part) => (part ?? "").trim()) | ||
| .filter(Boolean) | ||
| .join("\n\n") | ||
| }\n`; |
There was a problem hiding this comment.
Release-notes merge is not idempotent.
If this job is re-run for the same tag, release.data.body already contains prior generated notes, so this logic appends them again and duplicates content.
Suggested fix
- const body = `${
- [release.data.body, generated.data.body]
- .map((part) => (part ?? "").trim())
- .filter(Boolean)
- .join("\n\n")
- }\n`;
+ const existing = (release.data.body ?? "").trim();
+ const generatedBody = (generated.data.body ?? "").trim();
+ const merged =
+ existing && generatedBody && existing.includes(generatedBody)
+ ? existing
+ : [existing, generatedBody].filter(Boolean).join("\n\n");
+ const body = `${merged}\n`;📝 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.
| const body = `${ | |
| [release.data.body, generated.data.body] | |
| .map((part) => (part ?? "").trim()) | |
| .filter(Boolean) | |
| .join("\n\n") | |
| }\n`; | |
| const existing = (release.data.body ?? "").trim(); | |
| const generatedBody = (generated.data.body ?? "").trim(); | |
| const merged = | |
| existing && generatedBody && existing.includes(generatedBody) | |
| ? existing | |
| : [existing, generatedBody].filter(Boolean).join("\n\n"); | |
| const body = `${merged}\n`; |
🤖 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 @.github/scripts/release/notes.mjs around lines 26 - 31, The current assembly
of body using release.data.body and generated.data.body can duplicate generated
notes when the job reruns; update the logic that builds body (the body constant)
to first detect and remove any previously appended generated.data.body from
release.data.body (or check if release.data.body already includes
generated.data.body) before joining, so that joining release.data.body and
generated.data.body is idempotent and does not produce duplicated content.
| permissions: { contents: read, id-token: write } | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Arrr, ye scallywag! These permissions be sittin' at the wrong deck level! 🏴☠️
The id-token: write permission be legitimate treasure for the trusted publishin' with crates-io-auth-action, aye. But declarin' it at workflow level means every job in this vessel gets it, even if ye add more crew (jobs) later. The static analysis tool be right to hoist this flag.
Yer other release workflow (npm-release.yml) scopes permissions to the job level — be consistent with yer fleet, matey! With only one job currently, it's functionally the same, but job-level scopin' follows the principle of least privilege and prevents future mishaps.
🔒 Proposed fix to scope permissions to job level
-permissions: { contents: read, id-token: write }
-
env:
RELEASE_TAG: ${{ case(github.event_name == 'workflow_dispatch', inputs.tag, github.ref_name) }}
INPUT_DRY_RUN: ${{ inputs.dry-run || 'false' }}
@@ jobs:
publish:
name: publish to crates.io
runs-on: ubuntu-latest
+ # id-token: write required for rust-lang/crates-io-auth-action trusted publishing
+ permissions: { contents: read, id-token: write }
environment: { name: crates-io, url: "https://crates.io/crates/runner-run/${{ steps.verify.outputs.version }}" }📝 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.
| permissions: { contents: read, id-token: write } | |
| env: | |
| RELEASE_TAG: ${{ case(github.event_name == 'workflow_dispatch', inputs.tag, github.ref_name) }} | |
| INPUT_DRY_RUN: ${{ inputs.dry-run || 'false' }} | |
| jobs: | |
| publish: | |
| name: publish to crates.io | |
| runs-on: ubuntu-latest | |
| # id-token: write required for rust-lang/crates-io-auth-action trusted publishing | |
| permissions: { contents: read, id-token: write } | |
| environment: { name: crates-io, url: "https://crates.io/crates/runner-run/${{ steps.verify.outputs.version }}" } |
🧰 Tools
🪛 zizmor (1.25.2)
[error] 11-11: overly broad permissions (excessive-permissions): id-token: write is overly broad at the workflow level
(excessive-permissions)
[warning] 11-11: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment
(undocumented-permissions)
🤖 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 @.github/workflows/crates-release.yml around lines 11 - 12, The
workflow-level permissions block currently grants id-token: write to every job
(permissions: { contents: read, id-token: write }); move the id-token: write
permission out of the top-level permissions and add it only to the job that runs
the crates-io-auth-action / publishes to crates.io (the job that performs the
release/publish step), keeping contents: read at workflow level if needed or
scoping it to jobs that require it; update the job's permissions block to
include id-token: write and remove id-token from the global permissions to
enforce least privilege.
Source: Linters/SAST tools
| steps: | ||
| - name: publish github release | ||
| uses: actions/github-script@373c709c69115d41ff229c7e5df9f8788daa9553 # v9 | ||
| with: | ||
| retries: 3 | ||
| script: | | ||
| const { publishGithubRelease } = await import("${{ github.workspace }}/.github/scripts/release/publish.mjs"); | ||
| await publishGithubRelease({ core, github, context }); | ||
|
|
||
| - name: dispatch package publishes | ||
| uses: actions/github-script@373c709c69115d41ff229c7e5df9f8788daa9553 # v9 | ||
| with: | ||
| retries: 3 | ||
| script: | | ||
| const { dispatchPackagePublishes } = await import("${{ github.workspace }}/.github/scripts/release/publish.mjs"); | ||
| await dispatchPackagePublishes({ core, github, context }); |
There was a problem hiding this comment.
publish-release imports local scripts without checking out the repository.
This job dynamically imports files from ${{ github.workspace }}, but there is no checkout step in this job. The import will fail when the workspace does not contain repo files.
Suggested fix
publish-release:
name: publish github release
needs: [build-npm-dist]
if: github.event_name == 'push' && startsWith(github.ref, 'refs/tags/v')
runs-on: ubuntu-latest
permissions: { actions: write, contents: write }
steps:
+ - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
+ with: { persist-credentials: false }
+
- name: publish github release
uses: actions/github-script@373c709c69115d41ff229c7e5df9f8788daa9553 # v9
with:
retries: 3🧰 Tools
🪛 zizmor (1.25.2)
[warning] 234-234: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 242-242: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 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 @.github/workflows/release.yml around lines 228 - 243, The workflow step that
dynamically imports publishGithubRelease and dispatchPackagePublishes from "${{
github.workspace }}/.github/scripts/release/publish.mjs" can fail because the
repository files are not checked out; add a checkout step (e.g.,
actions/checkout) before the actions/github-script steps so that the workspace
contains the .github/scripts/release/publish.mjs file and the imports succeed;
ensure the checkout step appears prior to the steps invoking
publishGithubRelease and dispatchPackagePublishes.
| # build scripts | ||
| !.github/scripts/build | ||
| !.cargo |
There was a problem hiding this comment.
Block accidental Cargo credential commits.
Line 19 (!.cargo) re-exposes the whole .cargo tree; that can include token-bearing credentials files. Keep the directory unignored, but explicitly re-ignore credential files, matey.
Suggested fix
# build scripts
!.github/scripts/build
!.cargo
+.cargo/credentials
+.cargo/credentials.toml📝 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.
| # build scripts | |
| !.github/scripts/build | |
| !.cargo | |
| # build scripts | |
| !.github/scripts/build | |
| !.cargo | |
| .cargo/credentials | |
| .cargo/credentials.toml |
🤖 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 @.gitignore around lines 17 - 19, The current unignore line "!.cargo" exposes
the entire .cargo tree (risking token/credential commits); remove or stop
unignoring the whole directory and instead explicitly unignore only safe
artifacts while adding explicit ignore patterns for credentials. Update the
.gitignore by removing or replacing the "!.cargo" entry and add explicit ignores
such as ".cargo/credentials", ".cargo/credentials.toml", ".cargo/*.token",
".cargo/*.key", and any other credential filename patterns your project uses; if
you need to unignore specific safe files, unignore those exact filenames or
patterns rather than the entire .cargo directory.
| "fmt": "bunx dprint fmt" | ||
| }, | ||
| "devDependencies": { | ||
| "@actions/github-script": "github:actions/github-script#373c709c69115d41ff229c7e5df9f8788daa9553", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the `@actions/github-script` commit exists and identify its version
# Check if commit exists and get its details
gh api repos/actions/github-script/commits/373c709c69115d41ff229c7e5df9f8788daa9553 --jq '{sha: .sha, message: .commit.message, author: .commit.author.name, date: .commit.author.date}' || echo "Commit not found"
# Find which tags contain this commit
gh api repos/actions/github-script/tags --paginate --jq '.[] | select(.commit.sha == "373c709c69115d41ff229c7e5df9f8788daa9553") | {name: .name, sha: .commit.sha}'Repository: kjanat/runner
Length of output: 347
🏴☠️ Rip out the fake pin: the @actions/github-script SHA doesn’t exist.
package.json pins @actions/github-script to github:actions/github-script#373c709c69115d41ff229c7e5df9f8788daa9553, but GitHub’s API responds with HTTP 422 “No commit found” for that SHA in actions/github-script, so this reference can’t be audited. Point it at a commit that exists (or a proper release tag) instead.
🤖 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 `@package.json` at line 13, The dependency pin for "`@actions/github-script`" in
package.json is referencing a non-existent commit SHA; update the dependency
spec for "`@actions/github-script`" to a valid reference (either a published npm
version like a release tag e.g. "github-script@^6.1.0" or a real
commit/annotated tag from the actions/github-script repo) so installations and
audits succeed; locate the "`@actions/github-script`" entry in package.json and
replace the fake
"github:actions/github-script#373c709c69115d41ff229c7e5df9f8788daa9553" value
with a valid tag/commit or semver release.
1 participant
What
Ports the
.github/scriptsrelease tooling from shell to Node ESM. Seven.shscripts become nineactions/github-scriptmodules (.mjs), invokedfrom the workflows via dynamic
import():download-release-archives,man-archive,npm,package-release-asset,verify-checksumaur,npmnotes,publishEach module is
@ts-check'd with JSDoc types against@actions/github-scriptand receives the injected
core/github/context/exec, so thescripts carry no runtime dependencies. Also adds
.node-version, the@actions/github-script/@types/node/typescriptdevDeps, and thesupporting
package.json/tsconfig.json/bun.lock/.gitignoreupdates.
Verified
.mjspassnode --check.import(...)resolves to a realexport(12/12 checked)..shreferences remain in any workflow.This branch was authored against the
workflow_dispatchrelease designand is 5 commits behind master, which has since moved release chaining to
workflow_call(58ace23,84a987c). The two designs collide, so thisPR will show conflicts on the 4 workflow files and the 2
.shfiles mastermodified.
Reconciling onto master's
workflow_callarchitecture means:release/publish.mjs'sdispatchPackagePublishesis obsolete (master invokes publishers via
uses:, not API dispatch).publish/npm.mjs:resolveReleaseRun+waitForReleaseRun(~80 lines of poll-and-wait) are dead under
workflow_call, where thecaller passes
run-idand the build is already complete.download-release-archives,man-archive,npm(build),package-release-asset,verify-checksum,notes,aur, andnpm.mjs'sderiveNpmPublishSettings/publishNpmPackagescore) are orchestration-agnostic.Open decisions for the integration
workflow_callorchestration (assumed yes).workflow_call(after thefull build); this migration self-triggers crates on
push: tags. Differentordering/failure semantics — pick one.
release:[published]trigger path, or delete the wait loop outright.
Kept as a draft until the above is resolved.