From 7a71e01941d29c384d2638a3503a75750cce38d4 Mon Sep 17 00:00:00 2001 From: yen <5915590+antigenius0910@users.noreply.github.com> Date: Thu, 22 Oct 2026 21:34:55 -0500 Subject: [PATCH 1/2] chore(ci): harden node-security-check per #1571 review #1571 was merged before these review follow-ups landed. This applies the remaining suggestions from its round-1 review. - F5: report an image or pin on a different Node major than its line as "major mismatch" and count it as behind. Before this change, an image on a higher major than its Dockerfile line was reported "current". - F7: bound every curl call with connect and total timeouts. - F8: mark the tracker issue with , act only on labeled issues that carry the marker, and document the label and Dockerfile contracts next to the code that relies on them. - F9: exempt node-security issues from issue-check.yml, so a maintainer edit or relabel does not mark the tracker incomplete. Refs #1567 Co-Authored-By: Claude Opus 5.5 --- .github/scripts/node_security_check.sh | 27 ++++++++++++++++++-------- .github/workflows/issue-check.yml | 2 ++ 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/.github/scripts/node_security_check.sh b/.github/scripts/node_security_check.sh index 438678783..0ccaa6897 100644 --- a/.github/scripts/node_security_check.sh +++ b/.github/scripts/node_security_check.sh @@ -5,13 +5,18 @@ # everything is current. Exits non-zero only when the check itself breaks, so # stale data never turns the workflow red. See #1567. # +# Contract with the Dockerfile: each Node stage starts FROM node:${NODE_VERSION} +# or from another Node stage, and each non-base-* stage is published as -. +# # Env: DOCKERFILE (default Dockerfile.unified), DRY_RUN=1 prints the decision # and issue body without calling gh, GH_TOKEN for gh. set -euo pipefail shopt -s inherit_errexit DOCKERFILE=${DOCKERFILE:-Dockerfile.unified} +# close-unlinked-stale-issues.yml and issue-check.yml exempt this literal label; rename it in all three. LABEL='node-security' +MARKER='' REPO=openabdev/openab CHANNELS=(stable beta) MANIFEST_ACCEPT=application/vnd.oci.image.index.v1+json,application/vnd.docker.distribution.manifest.list.v2+json,application/vnd.oci.image.manifest.v1+json,application/vnd.docker.distribution.manifest.v2+json @@ -26,11 +31,14 @@ version_lt() { [[ $1 != "$2" && $(printf '%s\n%s\n' "$1" "$2" | sort -V | head - status_of() { if [[ -z $1 ]]; then echo "not published" + elif [[ ${1%%.*} != "$2" ]]; then echo "major mismatch" elif [[ -n ${LATEST[$2]} ]] && version_lt "$1" "${LATEST[$2]}"; then echo stale else echo current fi } +is_behind() { [[ $1 == stale || $1 == "major mismatch" ]]; } + parse_dockerfile() { [[ -r $DOCKERFILE ]] || die "cannot read $DOCKERFILE" local line stage m @@ -63,7 +71,7 @@ parse_dockerfile() { load_latest_security() { local index m - index=$(curl -fsSL --retry 3 https://nodejs.org/dist/index.json) || die "cannot fetch https://nodejs.org/dist/index.json" + index=$(curl -fsSL --retry 3 --connect-timeout 10 --max-time 30 https://nodejs.org/dist/index.json) || die "cannot fetch https://nodejs.org/dist/index.json" for m in "${MAJORS[@]}"; do LATEST[$m]=$(jq -r --arg p "v$m." '.[] | select(.security and (.version | startswith($p))) | .version | ltrimstr("v")' <<<"$index" | sort -V | tail -n1) done @@ -72,7 +80,7 @@ load_latest_security() { # Prints the response body, or nothing when the registry answers 404. registry_get() { local code - code=$(curl -sSL --retry 3 -o "$TMP/registry" -w '%{http_code}' \ + code=$(curl -sSL --retry 3 --connect-timeout 10 --max-time 30 -o "$TMP/registry" -w '%{http_code}' \ -H "Authorization: Bearer $REGISTRY_TOKEN" -H "Accept: $2" \ "https://ghcr.io/v2/$REPO/$1") || die "GET ghcr.io/v2/$REPO/$1 failed" case $code in @@ -99,17 +107,17 @@ image_node_version() { assess() { local m agent channel node status - REGISTRY_TOKEN=$(curl -fsS --retry 3 "https://ghcr.io/token?scope=repository:$REPO:pull" | jq -er .token) || + REGISTRY_TOKEN=$(curl -fsS --retry 3 --connect-timeout 10 --max-time 30 "https://ghcr.io/token?scope=repository:$REPO:pull" | jq -er .token) || die "cannot get an anonymous ghcr.io pull token" for m in "${MAJORS[@]}"; do PIN_STATUS[$m]=$(status_of "${PINNED[$m]}" "$m") - if [[ ${PIN_STATUS[$m]} == stale ]]; then STALE_MAJOR[$m]=1; fi + if is_behind "${PIN_STATUS[$m]}"; then STALE_MAJOR[$m]=1; fi for agent in ${AGENTS[$m]:-}; do for channel in "${CHANNELS[@]}"; do node=$(image_node_version "$channel-$agent") status=$(status_of "$node" "$m") IMAGE_ROWS+=("| \`$channel-$agent\` | ${node:--} | $status |") - if [[ $status == stale ]]; then STALE_MAJOR[$m]=1; fi + if is_behind "$status"; then STALE_MAJOR[$m]=1; fi done done done @@ -140,13 +148,14 @@ render_tables() { render_issue_body() { cat <_VERSION\` to the latest security release in every Dockerfile that declares it (\`git grep -lE 'ARG NODE[0-9]+_VERSION' -- 'Dockerfile*'\`). The smoke tests check that the values match. +1. For each stale pin, bump \`ARG NODE_VERSION\` to the latest security release in every Dockerfile that declares it (\`git grep -lE 'ARG NODE[0-9]+_VERSION' -- 'Dockerfile*'\`). \`$DOCKERFILE\` declares every line and each other Dockerfile declares only its own. The smoke tests read the expected versions from the Dockerfiles, so a bump needs no workflow edit. 2. Follow the "Base image security release" runbook in [RELEASING.md](https://github.com/$REPO/blob/main/RELEASING.md#base-image-security-release) to cut a beta, verify \`node --version\` in each \`beta-\` image, and promote to stable. 3. Background and policy: #1567. @@ -164,7 +173,7 @@ issue_title() { echo "Node.js security release pending: ${joined% / }" } -# (anything stale, labeled issue open) -> action. +# (anything stale, marked issue open) -> action. decide() { case "$1$2" in 10) echo create ;; @@ -183,7 +192,9 @@ ensure_label() { # Args: stale (0|1), title, body file. reconcile() { local issue action has_issue=0 - issue=$(gh issue list --label "$LABEL" --state open --limit 1 --json number --jq '.[0].number // empty') + # Only issues carrying MARKER are ours; a hand-labeled issue is left alone. + issue=$(gh issue list --label "$LABEL" --state open --json number,body \ + --jq "[.[] | select(.body | contains(\"$MARKER\"))][0].number // empty") if [[ -n $issue ]]; then has_issue=1; fi action=$(decide "$1" "$has_issue") echo "decision: $action${issue:+ (#$issue)}" diff --git a/.github/workflows/issue-check.yml b/.github/workflows/issue-check.yml index bfe821e00..afbdd2e53 100644 --- a/.github/workflows/issue-check.yml +++ b/.github/workflows/issue-check.yml @@ -20,6 +20,8 @@ jobs: const issue = context.payload.issue; const body = issue.body || ''; const labels = issue.labels.map(l => l.name); + // node-security issues are generated and kept current by node-security-check.yml. + if (labels.includes('node-security')) return; const marker = ''; const rules = { From 190186abfc244ef2dbd62a8bc22dc282583469cc Mon Sep 17 00:00:00 2001 From: chaodu-agent Date: Fri, 23 Oct 2026 19:54:57 -0400 Subject: [PATCH 2/2] =?UTF-8?q?chore(ci):=20address=20#1574=20review=20?= =?UTF-8?q?=E2=80=94=20mismatch=20title/next=20steps,=20reserved=20label?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - issue_title: fall back to 'Node major mismatch' when the line has no security release, so the title never ends empty or with ' / ' - issue body: intro and Next steps cover major-mismatch rows - LABEL comment: label is reserved for this script's tracker - reconcile: null-safe body match, explicit --limit - issue-check.yml: note GITHUB_TOKEN writes do not retrigger it --- .github/scripts/node_security_check.sh | 18 +++++++++++------- .github/workflows/issue-check.yml | 1 + 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/.github/scripts/node_security_check.sh b/.github/scripts/node_security_check.sh index 0ccaa6897..0f67d1111 100644 --- a/.github/scripts/node_security_check.sh +++ b/.github/scripts/node_security_check.sh @@ -14,7 +14,8 @@ set -euo pipefail shopt -s inherit_errexit DOCKERFILE=${DOCKERFILE:-Dockerfile.unified} -# close-unlinked-stale-issues.yml and issue-check.yml exempt this literal label; rename it in all three. +# Reserved for this script's tracker. close-unlinked-stale-issues.yml and issue-check.yml +# exempt it by label alone; rename it in all three files (this script and both workflows). LABEL='node-security' MARKER='' REPO=openabdev/openab @@ -149,15 +150,16 @@ render_tables() { render_issue_body() { cat <_VERSION\` to the latest security release in every Dockerfile that declares it (\`git grep -lE 'ARG NODE[0-9]+_VERSION' -- 'Dockerfile*'\`). \`$DOCKERFILE\` declares every line and each other Dockerfile declares only its own. The smoke tests read the expected versions from the Dockerfiles, so a bump needs no workflow edit. -2. Follow the "Base image security release" runbook in [RELEASING.md](https://github.com/$REPO/blob/main/RELEASING.md#base-image-security-release) to cut a beta, verify \`node --version\` in each \`beta-\` image, and promote to stable. -3. Background and policy: #1567. +2. A \`major mismatch\` row means the pin or image is on a different Node major than its line. Fix the \`ARG NODE_VERSION\` value, or rebuild the image from the correct line, instead of bumping the patch version. +3. Follow the "Base image security release" runbook in [RELEASING.md](https://github.com/$REPO/blob/main/RELEASING.md#base-image-security-release) to cut a beta, verify \`node --version\` in each \`beta-\` image, and promote to stable. +4. Background and policy: #1567. Last checked: $(date -u '+%Y-%m-%d %H:%M UTC') EOF @@ -166,7 +168,8 @@ EOF issue_title() { local m versions=() for m in "${MAJORS[@]}"; do - if [[ -n ${STALE_MAJOR[$m]:-} ]]; then versions+=("${LATEST[$m]}"); fi + # A line can be behind on major mismatch alone, with no security release to name. + if [[ -n ${STALE_MAJOR[$m]:-} ]]; then versions+=("${LATEST[$m]:-Node $m major mismatch}"); fi done local joined joined=$(printf '%s / ' "${versions[@]}") @@ -193,8 +196,9 @@ ensure_label() { reconcile() { local issue action has_issue=0 # Only issues carrying MARKER are ours; a hand-labeled issue is left alone. - issue=$(gh issue list --label "$LABEL" --state open --json number,body \ - --jq "[.[] | select(.body | contains(\"$MARKER\"))][0].number // empty") + # The label is reserved for this script, so at most one open issue is expected. + issue=$(gh issue list --label "$LABEL" --state open --limit 100 --json number,body \ + --jq "[.[] | select((.body // \"\") | contains(\"$MARKER\"))][0].number // empty") if [[ -n $issue ]]; then has_issue=1; fi action=$(decide "$1" "$has_issue") echo "decision: $action${issue:+ (#$issue)}" diff --git a/.github/workflows/issue-check.yml b/.github/workflows/issue-check.yml index afbdd2e53..125ccc63a 100644 --- a/.github/workflows/issue-check.yml +++ b/.github/workflows/issue-check.yml @@ -21,6 +21,7 @@ jobs: const body = issue.body || ''; const labels = issue.labels.map(l => l.name); // node-security issues are generated and kept current by node-security-check.yml. + // Its GITHUB_TOKEN writes do not trigger this workflow; this covers human edits and relabels. if (labels.includes('node-security')) return; const marker = '';