From 11ecaa77f3dc5f56b684040e5b6279a7279878c2 Mon Sep 17 00:00:00 2001 From: Steven Welch Date: Sun, 20 Sep 2026 09:09:45 -0600 Subject: [PATCH 1/6] feat: add isolated known-host source diagnostic --- .github/workflows/check-known-host-source.yml | 65 ++++++++++ docs/known-host-source-diagnostic.md | 38 ++++++ scripts/check-known-host-source.py | 58 +++++++++ tests/test-known-host-source.py | 111 ++++++++++++++++++ 4 files changed, 272 insertions(+) create mode 100644 .github/workflows/check-known-host-source.yml create mode 100644 docs/known-host-source-diagnostic.md create mode 100644 scripts/check-known-host-source.py create mode 100644 tests/test-known-host-source.py diff --git a/.github/workflows/check-known-host-source.yml b/.github/workflows/check-known-host-source.yml new file mode 100644 index 0000000..0b19b41 --- /dev/null +++ b/.github/workflows/check-known-host-source.yml @@ -0,0 +1,65 @@ +name: check-known-host-source + +on: + pull_request: + branches: + - main + workflow_dispatch: + inputs: + hero_host: + description: Approved SSH destination to match against the canonical source. + required: true + type: string + +permissions: + contents: read + +jobs: + synthetic-tests: + runs-on: ubuntu-24.04 + timeout-minutes: 5 + steps: + - name: Checkout + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 + with: + persist-credentials: false + - name: Set up Python + uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 + with: + python-version: "3.14" + - name: Test source diagnostic with synthetic material + run: python tests/test-known-host-source.py + + check-source: + if: github.event_name == 'workflow_dispatch' && github.ref == 'refs/heads/main' + needs: synthetic-tests + runs-on: arc-tf + environment: production + timeout-minutes: 5 + permissions: + contents: read + id-token: write + concurrency: + group: check-known-host-source + cancel-in-progress: false + steps: + - name: Checkout + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 + with: + persist-credentials: false + - name: Configure existing AWS SOPS KMS access + uses: aws-actions/configure-aws-credentials@e6de054238d6b7531b4efff3b6587d9aade6a06c + with: + role-to-assume: arn:aws:iam::332355796717:role/github-actions-sops-kms + aws-region: us-west-2 + role-duration-seconds: 900 + unset-current-credentials: true + - name: Match canonical known-host source without value output + timeout-minutes: 1 + env: + HERO_HOST: ${{ inputs.hero_host }} + run: python3 scripts/check-known-host-source.py + - name: Remove diagnostic temporary material + if: always() + shell: bash + run: rm -rf -- "$RUNNER_TEMP/known-host-source-check" diff --git a/docs/known-host-source-diagnostic.md b/docs/known-host-source-diagnostic.md new file mode 100644 index 0000000..7d0aff1 --- /dev/null +++ b/docs/known-host-source-diagnostic.md @@ -0,0 +1,38 @@ +# Canonical known-host source diagnostic + +`tfroot-github` maintains `scripts/check-known-host-source.py` and its sole consumer, the `check-known-host-source` workflow. This is an owner-approved diagnostic for the existing `ssh_known_hosts` distribution path, not a new secret distributor. + +The existing `arc-tf` image supplies Python, SOPS and OpenSSH. The helper delegates extraction and matching to SOPS and `ssh-keygen`; no alternate parser, remote key scan, new package, or shared-workflow fork is used. Thin orchestration suppresses their output and removes the temporary file. + +## Scope and gates + +PR events run synthetic tests only in this new workflow. Source extraction runs only on a manually approved dispatch from `main`, after the tests, using the existing `production` environment and AWS SOPS KMS role. Supply the approved destination as `hero_host`; do not commit it to this repository. No Actions secret is read back, no GitHub secret is written, no host is contacted, and no OpenTofu/state operation runs in this diagnostic. + +**The repository's existing OpenTofu workflow is unchanged.** Opening a PR still triggers its usual test/plan. Merging this diagnostic to `main` still triggers the usual main workflow, including its environment-scoped apply job. Review all outstanding infrastructure changes and actual environment protection before merge; installing this diagnostic must not be treated as approval to reapply unrelated infrastructure. A GitHub environment declaration alone does not prove that required-reviewer protection is configured. + +After a separately approved merge, use a new manual `check-known-host-source` dispatch from `main`. Do not rerun an old OpenTofu apply as a diagnostic. IAM/KMS access in this new job is not proven until the authorized dispatch succeeds. + +## Output contract + +The helper extracts the canonical `ssh_known_hosts` field from `secrets/secrets.yaml` inside CI, suppressing SOPS stdout/stderr from logs. A temporary file is held under `$RUNNER_TEMP/known-host-source-check`, directory mode 0700 and file mode 0600. OpenSSH matching output is discarded. Normal success/failure removes the temporary file and directory; an always-run workflow step provides cleanup after interruption. No artifacts or caches contain the extracted material. + +Only `source_known_hosts: status=...` is emitted: + +- `match-found`: OpenSSH found a matching host entry. This is not proof of key correctness, ordinary ED25519 validity, revocation status, remote identity, or equality with the consumer secret. +- `missing-host-entry`: OpenSSH found no matching destination in the extracted source. +- `source-extraction-failed`: SOPS could not extract the field. No underlying error content is printed. +- `source-unusable`: extraction was empty or exceeded 1 MiB. +- `invalid-input`: empty, dash-prefixed, whitespace/control-bearing target. +- `tool-error`: unavailable tool, timeout, temporary-file error or unexpected failure. + +Extraction is bounded to 30 seconds; the match probe to five seconds; the whole step to one minute. The byte limit is checked after capture, not a streaming memory bound. The workflow prints no key material, source contents, fingerprints, hashes, or tool exception details. GitHub may display the non-secret dispatch destination in step environment metadata. + +## Interpretation + +Compare the source result with `hero-host-config`'s runner-local preflight using the same destination: + +1. Source missing: inspect/correct the canonical encrypted field through the trusted owner editing path. Do not rotate the remote key merely because the destination is absent. +2. Source match but consumer missing: investigate the particular Actions-secret resource update and non-sensitive metadata. Neither this check nor a successful apply proves byte-for-byte delivery equality. +3. Both match: keep strict host verification enabled and continue the separately approved Hero check workflow. + +There are no changes to `secrets.tf`, `gh-secrets.tf`, encrypted sources, consumer mappings, or the Hero workflow. Rolling back removes the helper/test/workflow/docs through a reviewed PR; no secret or host rollback is needed. CI tests use generated synthetic keys and mocked extraction, never real SOPS/KMS access. diff --git a/scripts/check-known-host-source.py b/scripts/check-known-host-source.py new file mode 100644 index 0000000..aa9e226 --- /dev/null +++ b/scripts/check-known-host-source.py @@ -0,0 +1,58 @@ +#!/usr/bin/env python3 + +import os +import subprocess +import sys +import tempfile +from pathlib import Path + + +SOURCE_COMMAND = ["sops", "--decrypt", "--extract", '["ssh_known_hosts"]', "secrets/secrets.yaml"] +MAX_BYTES = 1024 * 1024 + + +def check(host, directory): + if not host or host.startswith("-") or any( + char.isspace() or ord(char) < 32 or ord(char) == 127 for char in host + ): + return "invalid-input" + try: + os.mkdir(directory, mode=0o700) + except OSError: + return "tool-error" + try: + source = subprocess.run(SOURCE_COMMAND, capture_output=True, timeout=30) + if source.returncode != 0: + return "source-extraction-failed" + if not source.stdout or len(source.stdout) > MAX_BYTES: + return "source-unusable" + with tempfile.NamedTemporaryFile(dir=directory) as known_hosts: + known_hosts.write(source.stdout) + known_hosts.flush() + matched = subprocess.run( + ["ssh-keygen", "-F", host, "-f", known_hosts.name], + stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, timeout=5, + ) + if matched.returncode == 0: + return "match-found" + if matched.returncode == 1: + return "missing-host-entry" + return "tool-error" + except (OSError, subprocess.TimeoutExpired): + return "tool-error" + finally: + os.rmdir(directory) + + +def main(): + try: + directory = Path(os.environ["RUNNER_TEMP"]) / "known-host-source-check" + status = check(os.environ.get("HERO_HOST", ""), directory) + except Exception: + status = "tool-error" + print(f"source_known_hosts: status={status}") + return 0 if status == "match-found" else 1 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/test-known-host-source.py b/tests/test-known-host-source.py new file mode 100644 index 0000000..58d78c4 --- /dev/null +++ b/tests/test-known-host-source.py @@ -0,0 +1,111 @@ +import contextlib +import importlib.util +import io +import os +import subprocess +import tempfile +import unittest +from pathlib import Path +from unittest import mock + +ROOT = Path(__file__).resolve().parents[1] +SPEC = importlib.util.spec_from_file_location("source_check", ROOT / "scripts/check-known-host-source.py") +CHECK = importlib.util.module_from_spec(SPEC) +SPEC.loader.exec_module(CHECK) +REAL_RUN = subprocess.run + + +class SourceCheckTests(unittest.TestCase): + def setUp(self): + temporary = tempfile.TemporaryDirectory() + self.addCleanup(temporary.cleanup) + self.root = Path(temporary.name) + self.directory = self.root / "known-host-source-check" + key = self.root / "fixture" + result = REAL_RUN(["ssh-keygen", "-q", "-t", "ed25519", "-N", "", "-C", "fixture", "-f", str(key)], + capture_output=True, timeout=10) + self.assertTrue(result.returncode == 0, "fixture generation failed") + self.value = b"example.test " + key.with_suffix(".pub").read_bytes() + + def run_check(self, value=None, extraction_code=0, parse_error=None): + source = self.value if value is None else value + before = set(self.root.iterdir()) + + def run(args, **kwargs): + if args[0] == "sops": + self.assertTrue(args == ["sops", "--decrypt", "--extract", '["ssh_known_hosts"]', "secrets/secrets.yaml"], + "wrong source extraction") + self.assertTrue(kwargs.get("capture_output") and kwargs.get("timeout") == 30, "unsafe extraction") + return subprocess.CompletedProcess(args, extraction_code, stdout=source, stderr=b"sensitive-sentinel") + self.assertTrue(args[:3] == ["ssh-keygen", "-F", "example.test"], "unexpected command") + self.assertTrue(kwargs.get("stdout") == subprocess.DEVNULL and kwargs.get("stderr") == subprocess.DEVNULL, + "probe output not suppressed") + self.assertTrue(kwargs.get("timeout") == 5, "probe unbounded") + path = Path(args[4]) + self.assertTrue(path.parent == self.directory, "wrong temporary location") + self.assertTrue(path.stat().st_mode & 0o777 == 0o600, "unsafe file mode") + self.assertTrue(self.directory.stat().st_mode & 0o777 == 0o700, "unsafe directory mode") + if parse_error: + raise parse_error + return REAL_RUN(args, **kwargs) + + stdout, stderr = io.StringIO(), io.StringIO() + with mock.patch.dict(os.environ, {"RUNNER_TEMP": str(self.root), "HERO_HOST": "example.test"}): + with mock.patch.object(CHECK.subprocess, "run", side_effect=run): + with contextlib.redirect_stdout(stdout), contextlib.redirect_stderr(stderr): + code = CHECK.main() + output = stdout.getvalue() + statuses = ("match-found", "missing-host-entry", "source-extraction-failed", "source-unusable", "tool-error") + self.assertTrue(output in {f"source_known_hosts: status={status}\n" for status in statuses}, "unexpected output") + self.assertTrue(stderr.getvalue() == "", "stderr disclosure") + for forbidden in ("example.test", "ssh-ed25519", "sensitive-sentinel", "SHA256:", str(self.root)): + self.assertTrue(forbidden not in output, "diagnostic disclosure") + self.assertTrue(set(self.root.iterdir()) == before, "temporary plaintext residue") + status = output.strip().split("=", 1)[1] + self.assertTrue(code == (0 if status == "match-found" else 1), "wrong exit code") + return status + + def test_match(self): + self.assertTrue(self.run_check() == "match-found", "valid entry not found") + + def test_missing_entry(self): + self.assertTrue(self.run_check(self.value.replace(b"example.test", b"other.test")) == "missing-host-entry", "wrong host matched") + + def test_hashed_entry(self): + fixture = self.root / "hashed" + fixture.write_bytes(self.value) + result = REAL_RUN(["ssh-keygen", "-H", "-f", str(fixture)], capture_output=True, timeout=5) + self.assertTrue(result.returncode == 0, "fixture hashing failed") + self.assertTrue(self.run_check(fixture.read_bytes()) == "match-found", "hashed host rejected") + + def test_extraction_failure(self): + self.assertTrue(self.run_check(extraction_code=1) == "source-extraction-failed", "extraction error misclassified") + + def test_empty_and_oversized(self): + for value in (b"", b"x" * (CHECK.MAX_BYTES + 1)): + self.assertTrue(self.run_check(value) == "source-unusable", "invalid size accepted") + + def test_probe_failure_cleanup(self): + for error in (FileNotFoundError("sensitive-sentinel"), subprocess.TimeoutExpired(["ssh-keygen"], 5)): + self.assertTrue(self.run_check(parse_error=error) == "tool-error", "tool error misclassified") + + def test_invalid_host(self): + for host in ("", "-F", "bad host", "bad\nhost", "bad\x00host"): + self.assertTrue(CHECK.check(host, self.directory) == "invalid-input", "invalid target accepted") + self.assertTrue(not self.directory.exists(), "unexpected temporary directory") + + def test_existing_directory_not_touched(self): + self.directory.mkdir() + sentinel = self.directory / "sentinel" + sentinel.write_text("untouched") + self.assertTrue(CHECK.check("example.test", self.directory) == "tool-error", "existing path accepted") + self.assertTrue(sentinel.read_text() == "untouched", "existing path modified") + + def test_sops_timeout_cleanup(self): + with mock.patch.object(CHECK.subprocess, "run", side_effect=subprocess.TimeoutExpired(["sops"], 30)): + self.assertTrue(CHECK.check("example.test", self.directory) == "tool-error", "extraction timeout misclassified") + self.assertTrue(not self.directory.exists(), "temporary directory remains") + + +if __name__ == "__main__": + unittest.main() From ec840d1554460edc8cfcc35097a9da853e976e20 Mon Sep 17 00:00:00 2001 From: Steven Welch Date: Sun, 20 Sep 2026 09:19:09 -0600 Subject: [PATCH 2/6] fix: harden known-host source diagnostic --- .github/workflows/check-known-host-source.yml | 2 +- docs/known-host-source-diagnostic.md | 2 +- scripts/check-known-host-source.py | 14 ++++- tests/test-known-host-source.py | 57 ++++++++++++++++++- 4 files changed, 67 insertions(+), 8 deletions(-) diff --git a/.github/workflows/check-known-host-source.yml b/.github/workflows/check-known-host-source.yml index 0b19b41..708a0c2 100644 --- a/.github/workflows/check-known-host-source.yml +++ b/.github/workflows/check-known-host-source.yml @@ -62,4 +62,4 @@ jobs: - name: Remove diagnostic temporary material if: always() shell: bash - run: rm -rf -- "$RUNNER_TEMP/known-host-source-check" + run: rm -rf -- "$RUNNER_TEMP/known-host-source-check-${GITHUB_RUN_ID}-${GITHUB_RUN_ATTEMPT}" diff --git a/docs/known-host-source-diagnostic.md b/docs/known-host-source-diagnostic.md index 7d0aff1..67b4b55 100644 --- a/docs/known-host-source-diagnostic.md +++ b/docs/known-host-source-diagnostic.md @@ -14,7 +14,7 @@ After a separately approved merge, use a new manual `check-known-host-source` di ## Output contract -The helper extracts the canonical `ssh_known_hosts` field from `secrets/secrets.yaml` inside CI, suppressing SOPS stdout/stderr from logs. A temporary file is held under `$RUNNER_TEMP/known-host-source-check`, directory mode 0700 and file mode 0600. OpenSSH matching output is discarded. Normal success/failure removes the temporary file and directory; an always-run workflow step provides cleanup after interruption. No artifacts or caches contain the extracted material. +The helper extracts the canonical `ssh_known_hosts` field from `secrets/secrets.yaml` inside CI, suppressing SOPS stdout/stderr from logs. A run-scoped temporary directory is held under `$RUNNER_TEMP/known-host-source-check-${GITHUB_RUN_ID}-${GITHUB_RUN_ATTEMPT}`, with directory mode 0700 and file mode 0600. OpenSSH matching output is discarded. Normal success/failure removes the temporary file and directory; an always-run workflow step provides best-effort cleanup after interruption, but cleanup is not guaranteed after a host crash. No artifacts or caches contain the extracted material. Only `source_known_hosts: status=...` is emitted: diff --git a/scripts/check-known-host-source.py b/scripts/check-known-host-source.py index aa9e226..f34e476 100644 --- a/scripts/check-known-host-source.py +++ b/scripts/check-known-host-source.py @@ -21,7 +21,9 @@ def check(host, directory): except OSError: return "tool-error" try: - source = subprocess.run(SOURCE_COMMAND, capture_output=True, timeout=30) + source = subprocess.run( + SOURCE_COMMAND, stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, timeout=30 + ) if source.returncode != 0: return "source-extraction-failed" if not source.stdout or len(source.stdout) > MAX_BYTES: @@ -41,12 +43,18 @@ def check(host, directory): except (OSError, subprocess.TimeoutExpired): return "tool-error" finally: - os.rmdir(directory) + try: + os.rmdir(directory) + except OSError: + pass def main(): try: - directory = Path(os.environ["RUNNER_TEMP"]) / "known-host-source-check" + directory = Path(os.environ["RUNNER_TEMP"]) / ( + f"known-host-source-check-{os.environ['GITHUB_RUN_ID']}-" + f"{os.environ['GITHUB_RUN_ATTEMPT']}" + ) status = check(os.environ.get("HERO_HOST", ""), directory) except Exception: status = "tool-error" diff --git a/tests/test-known-host-source.py b/tests/test-known-host-source.py index 58d78c4..b36cf18 100644 --- a/tests/test-known-host-source.py +++ b/tests/test-known-host-source.py @@ -20,7 +20,7 @@ def setUp(self): temporary = tempfile.TemporaryDirectory() self.addCleanup(temporary.cleanup) self.root = Path(temporary.name) - self.directory = self.root / "known-host-source-check" + self.directory = self.root / "known-host-source-check-123-1" key = self.root / "fixture" result = REAL_RUN(["ssh-keygen", "-q", "-t", "ed25519", "-N", "", "-C", "fixture", "-f", str(key)], capture_output=True, timeout=10) @@ -35,7 +35,9 @@ def run(args, **kwargs): if args[0] == "sops": self.assertTrue(args == ["sops", "--decrypt", "--extract", '["ssh_known_hosts"]', "secrets/secrets.yaml"], "wrong source extraction") - self.assertTrue(kwargs.get("capture_output") and kwargs.get("timeout") == 30, "unsafe extraction") + self.assertTrue(kwargs.get("stdout") == subprocess.PIPE and kwargs.get("stderr") == subprocess.DEVNULL, + "unsafe extraction output") + self.assertTrue("capture_output" not in kwargs and kwargs.get("timeout") == 30, "unsafe extraction") return subprocess.CompletedProcess(args, extraction_code, stdout=source, stderr=b"sensitive-sentinel") self.assertTrue(args[:3] == ["ssh-keygen", "-F", "example.test"], "unexpected command") self.assertTrue(kwargs.get("stdout") == subprocess.DEVNULL and kwargs.get("stderr") == subprocess.DEVNULL, @@ -50,7 +52,8 @@ def run(args, **kwargs): return REAL_RUN(args, **kwargs) stdout, stderr = io.StringIO(), io.StringIO() - with mock.patch.dict(os.environ, {"RUNNER_TEMP": str(self.root), "HERO_HOST": "example.test"}): + with mock.patch.dict(os.environ, {"RUNNER_TEMP": str(self.root), "GITHUB_RUN_ID": "123", + "GITHUB_RUN_ATTEMPT": "1", "HERO_HOST": "example.test"}): with mock.patch.object(CHECK.subprocess, "run", side_effect=run): with contextlib.redirect_stdout(stdout), contextlib.redirect_stderr(stderr): code = CHECK.main() @@ -94,6 +97,18 @@ def test_invalid_host(self): self.assertTrue(CHECK.check(host, self.directory) == "invalid-input", "invalid target accepted") self.assertTrue(not self.directory.exists(), "unexpected temporary directory") + def test_invalid_input_main_output_and_exit(self): + for host in ("", "-F", "bad host", "bad\nhost"): + stdout, stderr = io.StringIO(), io.StringIO() + with mock.patch.dict(os.environ, {"RUNNER_TEMP": str(self.root), "GITHUB_RUN_ID": "123", + "GITHUB_RUN_ATTEMPT": "1", "HERO_HOST": host}): + with contextlib.redirect_stdout(stdout), contextlib.redirect_stderr(stderr): + code = CHECK.main() + self.assertTrue(code == 1, "invalid input returned success") + self.assertTrue(stdout.getvalue() == "source_known_hosts: status=invalid-input\n", "invalid output changed") + self.assertTrue(stderr.getvalue() == "", "invalid input disclosed stderr") + self.assertTrue(not self.directory.exists(), "invalid input created temporary directory") + def test_existing_directory_not_touched(self): self.directory.mkdir() sentinel = self.directory / "sentinel" @@ -106,6 +121,42 @@ def test_sops_timeout_cleanup(self): self.assertTrue(CHECK.check("example.test", self.directory) == "tool-error", "extraction timeout misclassified") self.assertTrue(not self.directory.exists(), "temporary directory remains") + def test_cleanup_failure_is_safe(self): + stdout, stderr = io.StringIO(), io.StringIO() + with mock.patch.dict(os.environ, {"RUNNER_TEMP": str(self.root), "GITHUB_RUN_ID": "123", + "GITHUB_RUN_ATTEMPT": "1", "HERO_HOST": "example.test"}): + with mock.patch.object(CHECK.subprocess, "run", side_effect=[ + subprocess.CompletedProcess(CHECK.SOURCE_COMMAND, 0, stdout=self.value, stderr=b"sensitive-sentinel"), + subprocess.CompletedProcess(["ssh-keygen"], 0), + ]): + with mock.patch.object(CHECK.os, "rmdir", side_effect=OSError("sensitive-sentinel")): + with contextlib.redirect_stdout(stdout), contextlib.redirect_stderr(stderr): + code = CHECK.main() + self.assertTrue(code == 0, "cleanup failure changed result") + self.assertTrue(stdout.getvalue() == "source_known_hosts: status=match-found\n", "cleanup failure disclosed output") + self.assertTrue(stderr.getvalue() == "", "cleanup failure disclosed stderr") + self.assertTrue(self.directory.exists(), "cleanup failure unexpectedly removed directory") + self.directory.rmdir() + + def test_workflow_guardrails(self): + workflow = (ROOT / ".github/workflows/check-known-host-source.yml").read_text() + synthetic = workflow.split(" check-source:", 1)[0] + source = workflow.split(" check-source:", 1)[1] + self.assertTrue("if: github.event_name == 'workflow_dispatch' && github.ref == 'refs/heads/main'" in source, + "source job is not manual main-only") + self.assertTrue("needs: synthetic-tests" in source, "source job bypasses synthetic tests") + self.assertTrue("environment: production" in source and "id-token: write" in source, + "source job lost production/OIDC gate") + for forbidden in ("credentials", "production", "id-token", "secrets."): + self.assertTrue(forbidden not in synthetic, "synthetic job contains production material") + self.assertTrue("tofu" not in workflow.lower() and "make " not in workflow.lower() and "apply" not in workflow.lower(), + "workflow contains infrastructure commands") + for line in workflow.splitlines(): + if "uses:" in line: + self.assertTrue(line.strip().rsplit("@", 1)[-1].isalnum() and + len(line.strip().rsplit("@", 1)[-1]) == 40, + "workflow action is not pinned") + if __name__ == "__main__": unittest.main() From ee87250d739d66b217ea87c3a8fe72ae8bcfd5f6 Mon Sep 17 00:00:00 2001 From: Steven Welch Date: Sun, 20 Sep 2026 09:23:14 -0600 Subject: [PATCH 3/6] test: distinguish credential action from checkout setting --- tests/test-known-host-source.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test-known-host-source.py b/tests/test-known-host-source.py index b36cf18..4ae169e 100644 --- a/tests/test-known-host-source.py +++ b/tests/test-known-host-source.py @@ -147,7 +147,7 @@ def test_workflow_guardrails(self): self.assertTrue("needs: synthetic-tests" in source, "source job bypasses synthetic tests") self.assertTrue("environment: production" in source and "id-token: write" in source, "source job lost production/OIDC gate") - for forbidden in ("credentials", "production", "id-token", "secrets."): + for forbidden in ("configure-aws-credentials", "production", "id-token", "secrets."): self.assertTrue(forbidden not in synthetic, "synthetic job contains production material") self.assertTrue("tofu" not in workflow.lower() and "make " not in workflow.lower() and "apply" not in workflow.lower(), "workflow contains infrastructure commands") From 3486b4b15b5b89aaa609e9793eeafd1db66cde03 Mon Sep 17 00:00:00 2001 From: Steven Welch Date: Sun, 20 Sep 2026 09:33:41 -0600 Subject: [PATCH 4/6] fix: bound known-host source streaming output --- docs/known-host-source-diagnostic.md | 4 +- scripts/check-known-host-source.py | 90 +++++++++++++++++++++++++--- tests/test-known-host-source.py | 80 ++++++++++++++++++------- 3 files changed, 143 insertions(+), 31 deletions(-) diff --git a/docs/known-host-source-diagnostic.md b/docs/known-host-source-diagnostic.md index 67b4b55..7dad952 100644 --- a/docs/known-host-source-diagnostic.md +++ b/docs/known-host-source-diagnostic.md @@ -14,7 +14,7 @@ After a separately approved merge, use a new manual `check-known-host-source` di ## Output contract -The helper extracts the canonical `ssh_known_hosts` field from `secrets/secrets.yaml` inside CI, suppressing SOPS stdout/stderr from logs. A run-scoped temporary directory is held under `$RUNNER_TEMP/known-host-source-check-${GITHUB_RUN_ID}-${GITHUB_RUN_ATTEMPT}`, with directory mode 0700 and file mode 0600. OpenSSH matching output is discarded. Normal success/failure removes the temporary file and directory; an always-run workflow step provides best-effort cleanup after interruption, but cleanup is not guaranteed after a host crash. No artifacts or caches contain the extracted material. +The helper extracts the canonical `ssh_known_hosts` field from `secrets/secrets.yaml` inside CI without buffering unbounded SOPS stdout. It reads stdout incrementally with a selector and `os.read`, allowing at most `MAX_BYTES + 1` bytes in memory so an oversize result is detected and terminated. A run-scoped temporary directory is held under `$RUNNER_TEMP/known-host-source-check-${GITHUB_RUN_ID}-${GITHUB_RUN_ATTEMPT}`, with directory mode 0700 and file mode 0600. OpenSSH matching output is discarded. Normal success/failure removes the temporary file and directory; an always-run workflow step provides best-effort cleanup after interruption, but cleanup is not guaranteed after a host crash. No artifacts or caches contain the extracted material. Only `source_known_hosts: status=...` is emitted: @@ -25,7 +25,7 @@ Only `source_known_hosts: status=...` is emitted: - `invalid-input`: empty, dash-prefixed, whitespace/control-bearing target. - `tool-error`: unavailable tool, timeout, temporary-file error or unexpected failure. -Extraction is bounded to 30 seconds; the match probe to five seconds; the whole step to one minute. The byte limit is checked after capture, not a streaming memory bound. The workflow prints no key material, source contents, fingerprints, hashes, or tool exception details. GitHub may display the non-secret dispatch destination in step environment metadata. +Extraction is bounded to 30 seconds; the match probe to five seconds; the whole step to one minute. On timeout or oversize, the child is killed and waited for with a bounded cleanup timeout. The workflow prints no key material, source contents, fingerprints, hashes, or tool exception details. GitHub may display the non-secret dispatch destination in step environment metadata. ## Interpretation diff --git a/scripts/check-known-host-source.py b/scripts/check-known-host-source.py index f34e476..c9e461f 100644 --- a/scripts/check-known-host-source.py +++ b/scripts/check-known-host-source.py @@ -1,14 +1,92 @@ #!/usr/bin/env python3 import os +import selectors import subprocess import sys import tempfile +import time from pathlib import Path SOURCE_COMMAND = ["sops", "--decrypt", "--extract", '["ssh_known_hosts"]', "secrets/secrets.yaml"] MAX_BYTES = 1024 * 1024 +SOURCE_TIMEOUT = 30 +SOURCE_CLEANUP_TIMEOUT = 1 + + +def _stop_source(process): + if process is None: + return + try: + if process.poll() is None: + process.kill() + except OSError: + pass + try: + process.wait(timeout=SOURCE_CLEANUP_TIMEOUT) + except (OSError, subprocess.TimeoutExpired): + try: + process.kill() + except OSError: + pass + try: + process.wait(timeout=SOURCE_CLEANUP_TIMEOUT) + except (OSError, subprocess.TimeoutExpired): + pass + for stream in (getattr(process, "stdout", None), getattr(process, "stderr", None)): + if stream is not None: + try: + stream.close() + except OSError: + pass + + +def _extract_source(): + process = None + selector = None + source = bytearray() + stream_closed = False + try: + process = subprocess.Popen( + SOURCE_COMMAND, stdout=subprocess.PIPE, stderr=subprocess.DEVNULL + ) + selector = selectors.DefaultSelector() + selector.register(process.stdout, selectors.EVENT_READ) + deadline = time.monotonic() + SOURCE_TIMEOUT + while not stream_closed: + remaining = deadline - time.monotonic() + if remaining <= 0: + return None, "tool-error" + if not selector.select(remaining): + return None, "tool-error" + remaining_bytes = MAX_BYTES + 1 - len(source) + chunk = os.read(process.stdout.fileno(), remaining_bytes) + if not chunk: + stream_closed = True + continue + source.extend(chunk) + if len(source) > MAX_BYTES: + return None, "source-unusable" + remaining = deadline - time.monotonic() + if remaining <= 0: + return None, "tool-error" + try: + return_code = process.wait(timeout=remaining) + except subprocess.TimeoutExpired: + return None, "tool-error" + if return_code != 0: + return None, "source-extraction-failed" + return bytes(source), None + except (OSError, subprocess.TimeoutExpired): + return None, "tool-error" + finally: + if selector is not None: + try: + selector.close() + except OSError: + pass + _stop_source(process) def check(host, directory): @@ -21,15 +99,13 @@ def check(host, directory): except OSError: return "tool-error" try: - source = subprocess.run( - SOURCE_COMMAND, stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, timeout=30 - ) - if source.returncode != 0: - return "source-extraction-failed" - if not source.stdout or len(source.stdout) > MAX_BYTES: + source, error = _extract_source() + if error: + return error + if not source: return "source-unusable" with tempfile.NamedTemporaryFile(dir=directory) as known_hosts: - known_hosts.write(source.stdout) + known_hosts.write(source) known_hosts.flush() matched = subprocess.run( ["ssh-keygen", "-F", host, "-f", known_hosts.name], diff --git a/tests/test-known-host-source.py b/tests/test-known-host-source.py index 4ae169e..2ebc652 100644 --- a/tests/test-known-host-source.py +++ b/tests/test-known-host-source.py @@ -3,6 +3,7 @@ import io import os import subprocess +import sys import tempfile import unittest from pathlib import Path @@ -12,6 +13,7 @@ SPEC = importlib.util.spec_from_file_location("source_check", ROOT / "scripts/check-known-host-source.py") CHECK = importlib.util.module_from_spec(SPEC) SPEC.loader.exec_module(CHECK) +REAL_POPEN = subprocess.Popen REAL_RUN = subprocess.run @@ -26,19 +28,39 @@ def setUp(self): capture_output=True, timeout=10) self.assertTrue(result.returncode == 0, "fixture generation failed") self.value = b"example.test " + key.with_suffix(".pub").read_bytes() + self.source_file = self.root / "synthetic-source" + + def producer(self, mode, exit_code=0): + if mode == "chunks": + code = "import pathlib,sys; data=pathlib.Path(sys.argv[1]).read_bytes(); [sys.stdout.buffer.write(data[i:i+8192]) or sys.stdout.buffer.flush() for i in range(0,len(data),8192)]" + return [sys.executable, "-c", code, str(self.source_file)] + if mode == "oversize": + code = "import os; chunk=b'x'*65536\nwhile True: os.write(1,chunk)" + return [sys.executable, "-c", code] + if mode == "sleep": + code = "import time; time.sleep(2)" + return [sys.executable, "-c", code] + code = "import sys; sys.exit(int(sys.argv[1]))" + return [sys.executable, "-c", code, str(exit_code)] + + def source_popen(self, source, mode="chunks", exit_code=0, processes=None): + self.source_file.write_bytes(source) + + def popen(args, **kwargs): + if args == CHECK.SOURCE_COMMAND: + process = REAL_POPEN(self.producer(mode, exit_code), **kwargs) + if processes is not None: + processes.append(process) + return process + return REAL_POPEN(args, **kwargs) + + return popen def run_check(self, value=None, extraction_code=0, parse_error=None): source = self.value if value is None else value before = set(self.root.iterdir()) def run(args, **kwargs): - if args[0] == "sops": - self.assertTrue(args == ["sops", "--decrypt", "--extract", '["ssh_known_hosts"]', "secrets/secrets.yaml"], - "wrong source extraction") - self.assertTrue(kwargs.get("stdout") == subprocess.PIPE and kwargs.get("stderr") == subprocess.DEVNULL, - "unsafe extraction output") - self.assertTrue("capture_output" not in kwargs and kwargs.get("timeout") == 30, "unsafe extraction") - return subprocess.CompletedProcess(args, extraction_code, stdout=source, stderr=b"sensitive-sentinel") self.assertTrue(args[:3] == ["ssh-keygen", "-F", "example.test"], "unexpected command") self.assertTrue(kwargs.get("stdout") == subprocess.DEVNULL and kwargs.get("stderr") == subprocess.DEVNULL, "probe output not suppressed") @@ -54,9 +76,11 @@ def run(args, **kwargs): stdout, stderr = io.StringIO(), io.StringIO() with mock.patch.dict(os.environ, {"RUNNER_TEMP": str(self.root), "GITHUB_RUN_ID": "123", "GITHUB_RUN_ATTEMPT": "1", "HERO_HOST": "example.test"}): - with mock.patch.object(CHECK.subprocess, "run", side_effect=run): - with contextlib.redirect_stdout(stdout), contextlib.redirect_stderr(stderr): - code = CHECK.main() + with mock.patch.object(CHECK.subprocess, "Popen", + side_effect=self.source_popen(source, exit_code=extraction_code)): + with mock.patch.object(CHECK.subprocess, "run", side_effect=run): + with contextlib.redirect_stdout(stdout), contextlib.redirect_stderr(stderr): + code = CHECK.main() output = stdout.getvalue() statuses = ("match-found", "missing-host-entry", "source-extraction-failed", "source-unusable", "tool-error") self.assertTrue(output in {f"source_known_hosts: status={status}\n" for status in statuses}, "unexpected output") @@ -85,8 +109,15 @@ def test_extraction_failure(self): self.assertTrue(self.run_check(extraction_code=1) == "source-extraction-failed", "extraction error misclassified") def test_empty_and_oversized(self): - for value in (b"", b"x" * (CHECK.MAX_BYTES + 1)): - self.assertTrue(self.run_check(value) == "source-unusable", "invalid size accepted") + self.assertTrue(self.run_check(b"") == "source-unusable", "empty source accepted") + processes = [] + before = set(self.root.iterdir()) + with mock.patch.object(CHECK.subprocess, "Popen", + side_effect=self.source_popen(b"", mode="oversize", processes=processes)): + status = CHECK.check("example.test", self.directory) + self.assertTrue(status == "source-unusable", "oversized source accepted") + self.assertTrue(processes and processes[0].poll() is not None, "oversized source child remains running") + self.assertTrue(set(self.root.iterdir()) == before, "temporary plaintext residue") def test_probe_failure_cleanup(self): for error in (FileNotFoundError("sensitive-sentinel"), subprocess.TimeoutExpired(["ssh-keygen"], 5)): @@ -116,22 +147,27 @@ def test_existing_directory_not_touched(self): self.assertTrue(CHECK.check("example.test", self.directory) == "tool-error", "existing path accepted") self.assertTrue(sentinel.read_text() == "untouched", "existing path modified") - def test_sops_timeout_cleanup(self): - with mock.patch.object(CHECK.subprocess, "run", side_effect=subprocess.TimeoutExpired(["sops"], 30)): - self.assertTrue(CHECK.check("example.test", self.directory) == "tool-error", "extraction timeout misclassified") + def test_source_timeout_cleanup(self): + processes = [] + with mock.patch.object(CHECK, "SOURCE_TIMEOUT", 0.05): + with mock.patch.object(CHECK.subprocess, "Popen", + side_effect=self.source_popen(b"", mode="sleep", processes=processes)): + status = CHECK.check("example.test", self.directory) + self.assertTrue(status == "tool-error", "source timeout misclassified") + self.assertTrue(processes and processes[0].poll() is not None, "timed-out source child remains running") self.assertTrue(not self.directory.exists(), "temporary directory remains") def test_cleanup_failure_is_safe(self): + processes = [] stdout, stderr = io.StringIO(), io.StringIO() with mock.patch.dict(os.environ, {"RUNNER_TEMP": str(self.root), "GITHUB_RUN_ID": "123", "GITHUB_RUN_ATTEMPT": "1", "HERO_HOST": "example.test"}): - with mock.patch.object(CHECK.subprocess, "run", side_effect=[ - subprocess.CompletedProcess(CHECK.SOURCE_COMMAND, 0, stdout=self.value, stderr=b"sensitive-sentinel"), - subprocess.CompletedProcess(["ssh-keygen"], 0), - ]): - with mock.patch.object(CHECK.os, "rmdir", side_effect=OSError("sensitive-sentinel")): - with contextlib.redirect_stdout(stdout), contextlib.redirect_stderr(stderr): - code = CHECK.main() + with mock.patch.object(CHECK.subprocess, "Popen", + side_effect=self.source_popen(self.value, processes=processes)): + with mock.patch.object(CHECK.subprocess, "run", side_effect=lambda args, **kwargs: REAL_RUN(args, **kwargs)): + with mock.patch.object(CHECK.os, "rmdir", side_effect=OSError("sensitive-sentinel")): + with contextlib.redirect_stdout(stdout), contextlib.redirect_stderr(stderr): + code = CHECK.main() self.assertTrue(code == 0, "cleanup failure changed result") self.assertTrue(stdout.getvalue() == "source_known_hosts: status=match-found\n", "cleanup failure disclosed output") self.assertTrue(stderr.getvalue() == "", "cleanup failure disclosed stderr") From c42fa72e54851e43baa4c075fc321ffd7cc3ed79 Mon Sep 17 00:00:00 2001 From: Steven Welch Date: Sun, 20 Sep 2026 09:35:15 -0600 Subject: [PATCH 5/6] test: stabilize streaming producer fixture --- tests/test-known-host-source.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test-known-host-source.py b/tests/test-known-host-source.py index 2ebc652..1d5a04a 100644 --- a/tests/test-known-host-source.py +++ b/tests/test-known-host-source.py @@ -29,10 +29,11 @@ def setUp(self): self.assertTrue(result.returncode == 0, "fixture generation failed") self.value = b"example.test " + key.with_suffix(".pub").read_bytes() self.source_file = self.root / "synthetic-source" + self.source_file.write_bytes(b"") def producer(self, mode, exit_code=0): if mode == "chunks": - code = "import pathlib,sys; data=pathlib.Path(sys.argv[1]).read_bytes(); [sys.stdout.buffer.write(data[i:i+8192]) or sys.stdout.buffer.flush() for i in range(0,len(data),8192)]" + code = "import pathlib,sys; data=pathlib.Path(sys.argv[1]).read_bytes();\nfor i in range(0,len(data),8192): sys.stdout.buffer.write(data[i:i+8192]); sys.stdout.buffer.flush()" return [sys.executable, "-c", code, str(self.source_file)] if mode == "oversize": code = "import os; chunk=b'x'*65536\nwhile True: os.write(1,chunk)" From e1e1e97d7fda8ac1449e986bde0d46f0678ffd5b Mon Sep 17 00:00:00 2001 From: Steven Welch Date: Sun, 20 Sep 2026 09:40:12 -0600 Subject: [PATCH 6/6] test: cover nonzero streaming extraction --- tests/test-known-host-source.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/test-known-host-source.py b/tests/test-known-host-source.py index 1d5a04a..a55dcec 100644 --- a/tests/test-known-host-source.py +++ b/tests/test-known-host-source.py @@ -49,6 +49,10 @@ def source_popen(self, source, mode="chunks", exit_code=0, processes=None): def popen(args, **kwargs): if args == CHECK.SOURCE_COMMAND: + self.assertTrue(args == ["sops", "--decrypt", "--extract", '["ssh_known_hosts"]', "secrets/secrets.yaml"], + "wrong source extraction") + self.assertTrue(kwargs.get("stdout") == subprocess.PIPE and kwargs.get("stderr") == subprocess.DEVNULL, + "unsafe extraction output") process = REAL_POPEN(self.producer(mode, exit_code), **kwargs) if processes is not None: processes.append(process) @@ -78,7 +82,11 @@ def run(args, **kwargs): with mock.patch.dict(os.environ, {"RUNNER_TEMP": str(self.root), "GITHUB_RUN_ID": "123", "GITHUB_RUN_ATTEMPT": "1", "HERO_HOST": "example.test"}): with mock.patch.object(CHECK.subprocess, "Popen", - side_effect=self.source_popen(source, exit_code=extraction_code)): + side_effect=self.source_popen( + source, + mode="exit" if extraction_code else "chunks", + exit_code=extraction_code, + )): with mock.patch.object(CHECK.subprocess, "run", side_effect=run): with contextlib.redirect_stdout(stdout), contextlib.redirect_stderr(stderr): code = CHECK.main()