From 41e60a42708126f2af62eb66a7cbe14fbdbdbd7a Mon Sep 17 00:00:00 2001 From: scttbnsn <80784472+scttbnsn@users.noreply.github.com> Date: Thu, 20 Aug 2026 18:49:46 -0400 Subject: [PATCH 1/3] feat(workflows): add the shared star-chart refresh reusable workflow Replaces both retired star-chart engines org-wide. The chart becomes a first-party SVG generated from GitHub's own stargazer timestamps and committed into the consuming repository, so it needs no secret and makes no request at render time. That property is the point. A live route that loses its credential serves a plausible placeholder at HTTP 200 forever with nothing reporting red, which is exactly how drydock's chart sat broken. A committed artifact fails visibly or not at all. The generator is embedded in the workflow rather than checked out from a second repository, so a caller's SHA pin covers every line of behaviour with nothing resolved at run time. Verified against live data before committing: byte-identical output to the reference implementation for drydock at 238 stars and 3 API calls, and a clean no-op exit on a repo with a single star. - feat(workflows): starchart-refresh.yml, egress-blocked to api.github.com and github.com, contents: write as its only elevated scope - test(workflows): contract test covering the embedded generator, env-var input handling, the self-contained SVG, and the conditional commit-back - ci(validation): run the new contract test in standards validation - docs(onboarding): document the caller shape and why the artifact is committed rather than served --- .../tests/starchart_refresh_contract_test.py | 130 ++++++++++++ .github/workflows/standards-validation.yml | 1 + .github/workflows/starchart-refresh.yml | 195 ++++++++++++++++++ REPOSITORY_ONBOARDING.md | 13 ++ 4 files changed, 339 insertions(+) create mode 100644 .github/tests/starchart_refresh_contract_test.py create mode 100644 .github/workflows/starchart-refresh.yml diff --git a/.github/tests/starchart_refresh_contract_test.py b/.github/tests/starchart_refresh_contract_test.py new file mode 100644 index 0000000..eb7578f --- /dev/null +++ b/.github/tests/starchart_refresh_contract_test.py @@ -0,0 +1,130 @@ +from pathlib import Path +import re +import unittest + + +ROOT = Path(__file__).resolve().parents[2] +WORKFLOW = ROOT / ".github/workflows/starchart-refresh.yml" + + +class StarchartRefreshContractTest(unittest.TestCase): + def test_reusable_workflow_shape_and_narrow_permissions(self): + workflow = self.read_workflow() + + for expected in ( + " workflow_call:\n", + " branch:\n", + " required: true\n", + " output-path:\n", + " default: docs/assets/star-history.svg\n", + " max-pages:\n", + " type: number\n", + "permissions: {}", + " runs-on: ubuntu-24.04", + " timeout-minutes: 10", + "uses: step-security/harden-runner@", + "egress-policy: block", + "api.github.com:443", + "github.com:443", + ): + self.assertIn(expected, workflow) + + # contents: write is the whole point of this workflow, but it must be + # the ONLY elevated scope. A second write scope here would be a + # commit-back job that can also move issues, releases, or packages. + job_scopes = re.findall(r"^ (\w[\w-]*): write", workflow, re.MULTILINE) + self.assertEqual(job_scopes, ["contents"]) + + def test_generator_is_embedded_rather_than_fetched_at_run_time(self): + """The caller pins this file by SHA. Anything resolved at run time + escapes that pin, so the generator lives inline and the only network + reads are GitHub's own API.""" + workflow = self.read_workflow() + + self.assertIn("node --input-type=module - <<'GENERATOR'", workflow) + self.assertIn("application/vnd.github.star+json", workflow) + self.assertIn("https://api.github.com/", workflow) + + # No second repository checkout, and no curl/wget/npm pulling code in. + self.assertEqual(workflow.count("actions/checkout@"), 1) + self.assertNotIn("repository: CodesWhat/.github", workflow) + for forbidden in ("curl ", "wget ", "npx ", "npm install", "pip install"): + self.assertNotIn(forbidden, workflow) + + def test_untrusted_input_is_read_from_the_environment(self): + """Caller-controlled values reach the script as env vars, never as + ${{ }} interpolated into a shell or JavaScript body.""" + workflow = self.read_workflow() + + for expected in ( + "TARGET_REPO: ${{ github.repository }}", + "OUTPUT_PATH: ${{ inputs.output-path }}", + "MAX_PAGES: ${{ inputs.max-pages }}", + "TARGET_BRANCH: ${{ inputs.branch }}", + "const repo = process.env.TARGET_REPO", + "const out = process.env.OUTPUT_PATH", + ): + self.assertIn(expected, workflow) + + generator = workflow.split("<<'GENERATOR'", 1)[1].split("GENERATOR", 1)[0] + self.assertNotIn("${{", generator) + + def test_chart_is_self_contained_with_no_external_references(self): + """A committed artifact that reaches out at render time would + reintroduce exactly the silent failure this replaced.""" + workflow = self.read_workflow() + + self.assertIn(" +# with: +# branch: dev/v1.7 +# +# The generator is embedded rather than checked out from a second repository +# so that the caller's SHA pin covers every line of behaviour, with nothing +# resolved at run time. + +on: + workflow_call: + inputs: + branch: + description: Branch to read and commit the refreshed chart to. Never main under the strict release flow. + required: true + type: string + output-path: + description: Path the SVG is written to, relative to the repository root. + required: false + default: docs/assets/star-history.svg + type: string + max-pages: + description: Safety cap on stargazer pages fetched (100 stars per page). + required: false + default: 100 + type: number + +permissions: {} + +jobs: + refresh: + name: Star Chart Refresh + runs-on: ubuntu-24.04 + timeout-minutes: 10 + concurrency: + group: starchart-refresh-${{ github.repository }}-${{ inputs.branch }} + cancel-in-progress: false + permissions: + contents: write # Commit the regenerated SVG back to the caller's branch. + + steps: + - name: Harden runner + uses: step-security/harden-runner@bf7454d06d71f1098171f2acdf0cd4708d7b5920 # v2.20.0 + with: + egress-policy: block + allowed-endpoints: > + api.github.com:443 + github.com:443 + + - name: Check out the caller at the target branch + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + ref: ${{ inputs.branch }} + # Kept, unlike everywhere else in this org: the whole job is a + # commit-back, so the pushing credential has to survive checkout. + persist-credentials: true # zizmor: ignore[artipacked] + + - name: Generate the star-history SVG + env: + # Read via the environment, never interpolated into the script body. + TARGET_REPO: ${{ github.repository }} + OUTPUT_PATH: ${{ inputs.output-path }} + MAX_PAGES: ${{ inputs.max-pages }} + GITHUB_API_TOKEN: ${{ github.token }} + run: | + node --input-type=module - <<'GENERATOR' + import { mkdirSync, writeFileSync } from 'node:fs' + import { dirname } from 'node:path' + + const repo = process.env.TARGET_REPO + const out = process.env.OUTPUT_PATH + const maxPages = Number(process.env.MAX_PAGES) + const auth = process.env.GITHUB_API_TOKEN + + const api = async (path, accept) => { + const res = await fetch(`https://api.github.com/${path}`, { + headers: { + accept, + authorization: `Bearer ${auth}`, + 'user-agent': 'codeswhat-starchart', + 'x-github-api-version': '2022-11-28', + }, + }) + if (!res.ok) throw new Error(`GET ${path} -> ${res.status} ${res.statusText}`) + return res.json() + } + + const total = (await api(`repos/${repo}`, 'application/vnd.github+json')).stargazers_count + const pages = Math.min(Math.ceil(total / 100), maxPages) + if (pages < Math.ceil(total / 100)) { + console.log(`::warning::capping at ${maxPages} pages; ${total} stars needs ${Math.ceil(total / 100)}`) + } + + const stars = [] + for (let p = 1; p <= pages; p++) { + const page = await api( + `repos/${repo}/stargazers?per_page=100&page=${p}`, + 'application/vnd.github.star+json', + ) + for (const s of page) if (s.starred_at) stars.push(new Date(s.starred_at).getTime()) + } + stars.sort((a, b) => a - b) + + // Too few points to plot is a real state for a young repo, not a + // failure. Leaving the previous SVG in place beats committing an + // empty chart or reporting red on nothing being wrong. + if (stars.length < 2) { + console.log(`::notice::${repo} has ${stars.length} star(s); leaving the chart untouched`) + process.exit(0) + } + + const W = 800, H = 400, P = { t: 30, r: 30, b: 45, l: 60 } + const [t0, t1] = [stars[0], stars.at(-1)] + const x = (t) => P.l + ((t - t0) / (t1 - t0 || 1)) * (W - P.l - P.r) + const y = (n) => H - P.b - (n / stars.length) * (H - P.t - P.b) + + const pts = stars.map((t, i) => `${x(t).toFixed(1)},${y(i + 1).toFixed(1)}`) + const line = `M ${x(t0).toFixed(1)},${y(0).toFixed(1)} L ${pts.join(' L ')}` + const area = `${line} L ${x(t1).toFixed(1)},${(H - P.b).toFixed(1)} L ${x(t0).toFixed(1)},${(H - P.b).toFixed(1)} Z` + + const fmt = (ms) => new Date(ms).toISOString().slice(0, 7) + const ticks = 4 + const xLabels = Array.from({ length: ticks + 1 }, (_, i) => { + const t = t0 + ((t1 - t0) * i) / ticks + // Anchor the end labels inward so they can't clip the viewBox edges. + const anchor = i === 0 ? 'start' : i === ticks ? 'end' : 'middle' + return `${fmt(t)}` + }).join('\n ') + const yLabels = Array.from({ length: ticks + 1 }, (_, i) => { + const n = Math.round((stars.length * i) / ticks) + return ` + ${n}` + }).join('\n ') + + mkdirSync(dirname(out), { recursive: true }) + writeFileSync(out, ` + Star history for ${repo} + + + ${repo} · ${stars.length} stars + + ${yLabels} + + + + + ${xLabels} + + + `) + console.log(`${repo}: ${stars.length} stars, ${pages} API call(s) -> ${out}`) + GENERATOR + + - name: Commit the chart only when it actually changed + env: + OUTPUT_PATH: ${{ inputs.output-path }} + TARGET_BRANCH: ${{ inputs.branch }} + run: | + set -euo pipefail + # --porcelain rather than `git diff`, which reports clean for a + # path that is new and therefore still untracked on first run. + if [ -z "$(git status --porcelain -- "$OUTPUT_PATH")" ]; then + echo "::notice::chart unchanged; nothing to commit" + exit 0 + fi + git config user.name "github-actions[bot]" + git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + git add -- "$OUTPUT_PATH" + git commit -m "chore(docs): refresh the star-history chart" + git push origin "HEAD:$TARGET_BRANCH" diff --git a/REPOSITORY_ONBOARDING.md b/REPOSITORY_ONBOARDING.md index f0bfd3b..896dda6 100644 --- a/REPOSITORY_ONBOARDING.md +++ b/REPOSITORY_ONBOARDING.md @@ -215,6 +215,19 @@ Add these only when the behavior exists: with a clear enforced or advisory threshold. - [ ] Translation synchronization only for a repository with a translation source of truth and configured provider credentials. +- [ ] Star-history chart refresh for a public repository whose README carries a + Star History section. Call this repository's `starchart-refresh.yml` at a + pinned full commit SHA from a thin caller on `schedule` plus + `workflow_dispatch`, granting the job `contents: write` and passing the + active integration branch as `branch`. It regenerates a first-party SVG from + GitHub's stargazer timestamps and commits it only when the chart actually + changed. The chart is a committed artifact rather than a live route or a + third-party embed on purpose: it needs no secret and makes no request at + render time, so a stale one is visible and a missing one is a visibly broken + image, where a route that loses its credential serves a plausible placeholder + at HTTP 200 indefinitely. Do not embed `star-history.com` or `warpchart.dev`; + both are retired organization-wide, and adopting the chart means removing + what it replaced in the same change. ### Qlty From 5525f5978a00c3f0fae76c84838935d2ea8899ce Mon Sep 17 00:00:00 2001 From: scttbnsn <80784472+scttbnsn@users.noreply.github.com> Date: Thu, 20 Aug 2026 18:53:41 -0400 Subject: [PATCH 2/3] test(workflows): syntax-check the embedded star-chart generator This workflow never runs in this repository, so a syntax error inside the heredoc would first surface in a consumer's scheduled job, days later and in someone else's lane. The test recovers the generator the way the shell will actually see it, stripping the run block's base indentation rather than reading the file as written, since a heredoc body that looks correct in YAML can still reach node malformed. Then node --check parses it. Verified with a negative control rather than assumed: injecting a syntax error into the generator fails the test, and reverting passes it. --- .../tests/starchart_refresh_contract_test.py | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/.github/tests/starchart_refresh_contract_test.py b/.github/tests/starchart_refresh_contract_test.py index eb7578f..dd91ab7 100644 --- a/.github/tests/starchart_refresh_contract_test.py +++ b/.github/tests/starchart_refresh_contract_test.py @@ -1,5 +1,8 @@ from pathlib import Path import re +import shutil +import subprocess +import tempfile import unittest @@ -110,6 +113,26 @@ def test_too_few_stars_is_a_clean_exit_not_a_failure(self): self.assertNotIn("process.exit(1)", workflow) self.assertNotIn("process.exit(2)", workflow) + def test_embedded_generator_is_valid_javascript(self): + """This workflow never runs in this repository, so a syntax error in + the heredoc would first surface in a consumer's scheduled job. Parse + it here instead.""" + node = shutil.which("node") + if node is None: + self.skipTest("node is not available") + + source = self.read_generator() + with tempfile.TemporaryDirectory() as tmp: + script = Path(tmp) / "generator.mjs" + script.write_text(source) + result = subprocess.run( + [node, "--check", str(script)], + capture_output=True, + text=True, + check=False, + ) + self.assertEqual(result.returncode, 0, result.stderr) + def test_workflow_pins_actions_and_is_run_by_standards_validation(self): workflow = self.read_workflow() actions = re.findall(r"^\s+uses: ([^\s#]+)", workflow, re.MULTILINE) @@ -125,6 +148,23 @@ def read_workflow(self): self.assertTrue(WORKFLOW.is_file(), f"missing workflow: {WORKFLOW}") return WORKFLOW.read_text() + def read_generator(self): + """Recover the generator exactly as the shell will see it: YAML strips + the run block's base indentation, so a heredoc body that only looks + right in the file can still reach node malformed.""" + workflow = self.read_workflow() + opener = "node --input-type=module - <<'GENERATOR'\n" + self.assertIn(opener, workflow) + + indent = " " * (len(workflow.split(opener)[0].rsplit("\n", 1)[-1])) + self.assertTrue(indent, "expected the run block to be indented") + + body = workflow.split(opener, 1)[1].split(f"\n{indent}GENERATOR", 1)[0] + return "\n".join( + line[len(indent):] if line.startswith(indent) else line + for line in body.split("\n") + ) + if __name__ == "__main__": unittest.main() From 8288ada60e169c3b968cd7ca38a7c6491dec7c6c Mon Sep 17 00:00:00 2001 From: scttbnsn <80784472+scttbnsn@users.noreply.github.com> Date: Thu, 20 Aug 2026 18:56:23 -0400 Subject: [PATCH 3/3] fix(workflows): reject the inputs that would publish a wrong star chart All three from CodeRabbit on #32, and the max-pages one was a real bug of exactly the kind this workflow exists to prevent. max-pages: 0 made pages 0, which fetched nothing, which hit the "too few stars" clean exit. A repository with 238 stars would have reported a green no-op. A cap below the needed page count was worse than that: it drew a chart from the first N pages and published a partial history as a whole one behind a ::warning:: nobody reads. Both now fail loudly, and the cap must be a positive integer. branch had no runtime guard. Omitting a default only prevents omission, so a caller could still pass main and, on a repository whose ruleset let the push through, commit straight to the default branch. Rejected before checkout rather than at the push, where the error would be confusing. output-path was read through the environment, which stops script injection but not traversal. An absolute or ../ path reached writeFileSync outside the checkout, and the commit step then found nothing staged and reported success. Writes now use the resolved and validated path rather than the raw input, since a check that doesn't govern the write is decoration. Verified behaviourally, not by reading: each rejected input throws, the one-star exit still no-ops, nothing lands outside the workspace, and the happy path is still byte-identical to the reference output for drydock. --- .../tests/starchart_refresh_contract_test.py | 33 +++++++++++++ .github/workflows/starchart-refresh.yml | 49 ++++++++++++++++--- 2 files changed, 76 insertions(+), 6 deletions(-) diff --git a/.github/tests/starchart_refresh_contract_test.py b/.github/tests/starchart_refresh_contract_test.py index dd91ab7..1454c19 100644 --- a/.github/tests/starchart_refresh_contract_test.py +++ b/.github/tests/starchart_refresh_contract_test.py @@ -103,6 +103,39 @@ def test_commit_back_is_conditional_and_never_targets_a_protected_branch(self): for forbidden in ("--force", "--no-verify", "git tag", "gh pr merge"): self.assertNotIn(forbidden, workflow) + def test_a_default_branch_target_is_rejected_before_checkout(self): + """Omitting a default only prevents omission. A caller can still pass + main, and on a repo whose ruleset lets the push through that would + commit straight to the default branch.""" + workflow = self.read_workflow() + + guard = workflow.split("Reject a protected branch", 1)[1].split("- name:", 1)[0] + for branch in ("main", "master", "HEAD"): + self.assertIn(branch, guard) + self.assertIn("exit 1", guard) + self.assertIn("${TARGET_BRANCH#refs/heads/}", guard) + + # The guard is worthless after the checkout has already happened. + self.assertLess( + workflow.index("Reject a protected branch"), + workflow.index("actions/checkout@"), + ) + + def test_generator_rejects_traversal_and_a_non_positive_page_cap(self): + """max-pages: 0 previously produced an empty star list, which took the + 'too few stars' exit and reported a clean no-op for a repository that + actually has stars.""" + workflow = self.read_workflow() + + self.assertIn("relative(workspace, target).startsWith('..')", workflow) + self.assertIn("isAbsolute(out)", workflow) + self.assertIn("!Number.isInteger(maxPages) || maxPages < 1", workflow) + + # Truncation must fail rather than publish a partial history. + self.assertIn("if (pages > maxPages)", workflow) + self.assertNotIn("Math.min(Math.ceil(total / 100), maxPages)", workflow) + self.assertNotIn("::warning::capping", workflow) + def test_too_few_stars_is_a_clean_exit_not_a_failure(self): """A young repo having one star is a real state, not a broken build. Reporting red there trains people to ignore the signal.""" diff --git a/.github/workflows/starchart-refresh.yml b/.github/workflows/starchart-refresh.yml index 1a8bcb6..8517969 100644 --- a/.github/workflows/starchart-refresh.yml +++ b/.github/workflows/starchart-refresh.yml @@ -63,6 +63,22 @@ jobs: api.github.com:443 github.com:443 + - name: Reject a protected branch as the commit-back target + env: + TARGET_BRANCH: ${{ inputs.branch }} + run: | + set -euo pipefail + # Omitting a default only prevents omission. Under the strict + # release flow nothing commits back to a default branch, and a + # named rejection here beats a confusing push rejection at the end + # of the job, or none at all on a repo that forgot its ruleset. + case "${TARGET_BRANCH#refs/heads/}" in + main | master | HEAD | "") + echo "::error::refusing to commit the chart to '$TARGET_BRANCH'; pass an integration branch such as dev/v1.7" >&2 + exit 1 + ;; + esac + - name: Check out the caller at the target branch uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: @@ -81,12 +97,24 @@ jobs: run: | node --input-type=module - <<'GENERATOR' import { mkdirSync, writeFileSync } from 'node:fs' - import { dirname } from 'node:path' + import { dirname, isAbsolute, relative, resolve } from 'node:path' const repo = process.env.TARGET_REPO const out = process.env.OUTPUT_PATH const maxPages = Number(process.env.MAX_PAGES) const auth = process.env.GITHUB_API_TOKEN + const workspace = process.env.GITHUB_WORKSPACE + + // Reading through the environment stops script injection but not + // path traversal. A write outside the checkout is worse than an + // error: the commit step finds nothing staged and reports success. + const target = resolve(workspace, out) + if (isAbsolute(out) || relative(workspace, target).startsWith('..')) { + throw new Error(`output-path must stay inside the repository: ${out}`) + } + if (!Number.isInteger(maxPages) || maxPages < 1) { + throw new Error(`max-pages must be a positive integer, got: ${process.env.MAX_PAGES}`) + } const api = async (path, accept) => { const res = await fetch(`https://api.github.com/${path}`, { @@ -102,9 +130,15 @@ jobs: } const total = (await api(`repos/${repo}`, 'application/vnd.github+json')).stargazers_count - const pages = Math.min(Math.ceil(total / 100), maxPages) - if (pages < Math.ceil(total / 100)) { - console.log(`::warning::capping at ${maxPages} pages; ${total} stars needs ${Math.ceil(total / 100)}`) + const pages = Math.ceil(total / 100) + // Truncating is a hard failure, not a warning. A chart drawn from + // the first N pages is a partial history rendered as a whole one, + // and a log line nobody reads is how that ships unnoticed. + if (pages > maxPages) { + throw new Error( + `${repo} has ${total} stars needing ${pages} pages but max-pages is ${maxPages}; ` + + 'raise the cap rather than publishing a truncated history', + ) } const stars = [] @@ -148,8 +182,11 @@ jobs: ${n}` }).join('\n ') - mkdirSync(dirname(out), { recursive: true }) - writeFileSync(out, ` + // Write the path that was validated, not the raw input. In Actions + // the working directory is the workspace so they agree, but a + // check that doesn't govern the write is decoration. + mkdirSync(dirname(target), { recursive: true }) + writeFileSync(target, ` Star history for ${repo}