From 83292af6ef6d45f533a1014ab083ca9f0f113ee5 Mon Sep 17 00:00:00 2001 From: rldyourmnd Date: Mon, 24 Aug 2026 03:11:12 +0500 Subject: [PATCH] feat(security): scan this module, and make its own pin check unfoolable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two things, found one because of the other. **This repository has never been scanned.** `GET /code-scanning/analyses` returns `no analysis found` and default setup is `not-configured`. Python is 75% of the tree and `actions` covers seven deployment workflows that hold apply and rollback authority. Four of the six public modules are in this state; the other three are being fixed alongside. **The catalog parser accepted a file no YAML parser will load.** Adding the CodeQL caller meant declaring it in `catalog/actions.yml`, and the entry went in mis-indented by two spaces. `yaml.safe_load` refuses that file outright — `expected , but found '-'` — and `validate_module.sh` printed OK. The reason is in the code this commit replaces: it stripped every line and looked for `- name:` anywhere, so indentation carried no meaning. A structurally broken catalog read "fine", and every pin comparison below it ran against whatever that tolerant scan happened to produce. The check could not tell a valid catalog from a corrupt one — the same trap as a duplicate YAML key that `safe_load` swallows while the real validator refuses it. The module ships no dependencies and CI installs none, so the fix is not PyYAML. It reads by column and refuses any other shape, plus two invariants worth stating: an empty registry is a failure rather than a vacuous pass, and a SHA that is not 40 characters is a failure. Each guard was proved to refuse bad input rather than merely to pass on good: mis-indented entry -> catalog/actions.yml:23: catalog entries are exactly … 12-character SHA -> records a 12-character SHA, expected 40 empty actions list -> declares no actions; refusing to pass a check with nothing to check and the corrected catalog passes. Pinned to `0.1.4` at the commit that tag resolves to; see NDDev-OpenNetwork/ci-workflows#45 for why the estate's other pins cannot be copied. Verified with `scripts/validate_module.sh` (10 contract tests OK) and actionlint 1.7.12 clean. --- .github/workflows/codeql.yml | 42 ++++++++++++++++++++++++++++++++++++ catalog/actions.yml | 3 +++ scripts/validate_module.sh | 42 +++++++++++++++++++++++++++++------- 3 files changed, 79 insertions(+), 8 deletions(-) create mode 100644 .github/workflows/codeql.yml diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml new file mode 100644 index 0000000..33957de --- /dev/null +++ b/.github/workflows/codeql.yml @@ -0,0 +1,42 @@ +# Code scanning for this repository. Advanced setup: the caller below is the +# only scanner, and CodeQL default setup is deliberately left `not-configured` +# because default setup cannot express a pinned reusable and cannot be reviewed +# in a diff. Attachment is atomic — enabling default setup later means disabling +# this workflow first, or the whole attachment fails. +# +# Verify rather than trust this comment: +# GET /repos/{owner}/{repo}/code-scanning/default-setup -> not-configured +# GET /repos/{owner}/{repo}/actions/workflows -> this file's state +name: codeql + +on: + push: + branches: [main] + pull_request: + schedule: + # Weekly, so a new query release is applied to unchanged code. Without this + # a repository that stops changing also stops being scanned. + - cron: '0 5 * * 2' + +permissions: {} + +concurrency: + group: codeql-${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + +jobs: + analyze: + name: codeql + permissions: + actions: read + contents: read + security-events: write + uses: NDDev-OpenNetwork/ci-workflows/.github/workflows/public-codeql.yml@b50364e2a415267688c1d845cea6866cdb5e53d6 # 0.1.4 + with: + # Public repository: `pull_request` runs untrusted fork code. Name the + # hosted runner explicitly — the reusable's default belongs to the pinned + # commit, not to this repository, so inheriting it would let a pin bump + # move fork pull requests onto private infrastructure with no diff here. + runner: ubuntu-latest + # Python is 75% of this repository; `actions` covers seven deployment workflows that hold apply and rollback authority. + languages: '["python","actions"]' diff --git a/catalog/actions.yml b/catalog/actions.yml index 7ff3d1f..b569bba 100644 --- a/catalog/actions.yml +++ b/catalog/actions.yml @@ -20,3 +20,6 @@ actions: - name: actions/upload-artifact sha: "043fb46d1a93c77aae656e7c1c64a875d1fc6a0a" version: "v7.0.1" + - name: NDDev-OpenNetwork/ci-workflows/.github/workflows/public-codeql.yml + sha: "b50364e2a415267688c1d845cea6866cdb5e53d6" + version: "0.1.4" diff --git a/scripts/validate_module.sh b/scripts/validate_module.sh index 8df9b16..558a45e 100755 --- a/scripts/validate_module.sh +++ b/scripts/validate_module.sh @@ -93,20 +93,46 @@ python3 - <<'PYCHECK' from pathlib import Path import re +# Read strictly, by column. The first version of this stripped every line and +# looked for `- name:` anywhere, which ignores indentation entirely -- so a +# catalog that no YAML parser will load still "read fine" and the whole check +# below passed on it. That was found by mis-indenting an entry by two spaces: +# `yaml.safe_load` raised `expected `, and this script printed OK. +# The module ships no dependencies, so the answer is not PyYAML; it is refusing +# any shape other than the one shape this file is allowed to have. registry = {} current = {} -for line in Path("catalog/actions.yml").read_text(encoding="utf-8").splitlines(): - stripped = line.strip() - if stripped.startswith("- name:"): +catalog = Path("catalog/actions.yml") +in_actions = False +for number, line in enumerate(catalog.read_text(encoding="utf-8").splitlines(), 1): + if not line.strip() or line.lstrip().startswith("#"): + continue + if not in_actions: + in_actions = line == "actions:" + continue + if line.startswith(" - name: "): if current: registry[current["name"]] = current - current = {"name": stripped.split(":", 1)[1].strip().strip('"')} - elif stripped.startswith("sha:") and current: - current["sha"] = stripped.split(":", 1)[1].strip().strip('"') - elif stripped.startswith("version:") and current: - current["version"] = stripped.split(":", 1)[1].strip().strip('"') + current = {"name": line[len(" - name: "):].strip().strip('"')} + elif line.startswith(" sha: ") and current: + current["sha"] = line[len(" sha: "):].strip().strip('"') + elif line.startswith(" version: ") and current: + current["version"] = line[len(" version: "):].strip().strip('"') + else: + raise SystemExit( + f"{catalog}:{number}: catalog entries are exactly " + f'` - name: X` / ` sha: \"…\"` / ` version: \"…\"`; got {line!r}' + ) if current: registry[current["name"]] = current +if not registry: + raise SystemExit(f"{catalog}: declares no actions; refusing to pass a check with nothing to check") +for name, entry in sorted(registry.items()): + missing = [k for k in ("sha", "version") if k not in entry] + if missing: + raise SystemExit(f"{catalog}: {name} is missing {', '.join(missing)}") + if len(entry["sha"]) != 40: + raise SystemExit(f"{catalog}: {name} records a {len(entry['sha'])}-character SHA, expected 40") pattern = re.compile(r"uses:\s+([A-Za-z0-9_.-]+/[A-Za-z0-9_./-]+)@([0-9a-f]{40})\s*#\s*(\S+)") seen = set()