fix(ci): validate untrusted workflow identifiers

scripts/ci/test_validate_coverage_artifact.py

@@ -24,12 +24,13 @@ def setUp(self):
         (self.root / "src").mkdir()
         (self.root / "internal").mkdir()
         (self.root / "src/a.ts").write_text("export const a = 1;\n")
+        (self.root / "-tracked.ts").write_text("export const tracked = true;\n")
         (self.root / "internal/a.go").write_text("package internal\n")
         (self.root / "go.mod").write_text("module github.com/RandomCodeSpace/kb\n\ngo 1.24\n")
         subprocess.run(["git", "init", "-q", self.root], check=True)
         subprocess.run(["git", "-C", self.root, "config", "user.email", "[email protected]"], check=True)
         subprocess.run(["git", "-C", self.root, "config", "user.name", "CI"], check=True)
-        subprocess.run(["git", "-C", self.root, "add", "src/a.ts", "internal/a.go", "go.mod"], check=True)
+        subprocess.run(["git", "-C", self.root, "add", "--", "src/a.ts", "-tracked.ts", "internal/a.go", "go.mod"], check=True)
         subprocess.run(["git", "-C", self.root, "commit", "-qm", "fixture"], check=True)
         self.base = subprocess.check_output(["git", "-C", self.root, "rev-parse", "HEAD"], text=True).strip()
         self.lcov = b"TN:\nSF:src/a.ts\nDA:1,1\nend_of_record\n"
@@ -84,24 +85,59 @@ def archive(self, *, lcov=None, go=None, manifest_bytes=None, symlink=False):
             archive.writestr("manifest.json", manifest_bytes)
         return path
 
-    def run_validator(self, archive, output=None):
+    def run_validator(self, archive, output=None, *, candidate_sha=SHA, base_sha=None, event="pull_request", pull_request=7):
         output = output or self.root / f"out-{os.urandom(4).hex()}"
+        base_sha = self.base if base_sha is None else base_sha
         args = [
             "python3", str(SCRIPT), "--archive", str(archive), "--output", str(output),
             "--repository", "RandomCodeSpace/kb", "--workflow", "Regression and candidate coverage",
             "--workflow-ref", "RandomCodeSpace/kb/.github/workflows/quality.yml@refs/pull/7/merge", "--workflow-sha", WORKFLOW_SHA,
             "--test-revision-sha", WORKFLOW_SHA,
-            "--run-id", "10", "--run-attempt", "1", "--event", "pull_request",
-            "--candidate-repository", "RandomCodeSpace/kb", "--candidate-sha", SHA,
-            "--candidate-tree", TREE, "--candidate-ref", "feature", "--pull-request", "7",
-            "--base-sha", self.base, "--security-base-sha", self.base, "--base-ref", "main", "--repository-root", str(self.root),
+            "--run-id", "10", "--run-attempt", "1", "--event", event,
+            "--candidate-repository", "RandomCodeSpace/kb", f"--candidate-sha={candidate_sha}",
+            "--candidate-tree", TREE, "--candidate-ref", "feature", "--pull-request", str(pull_request),
+            f"--base-sha={base_sha}", "--security-base-sha", self.base, "--base-ref", "main", "--repository-root", str(self.root),
             "--maintenance", "false",
         ]
         return subprocess.run(args, text=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
 
     def test_accepts_exact_bundle(self):
         self.assertEqual(self.run_validator(self.archive()).returncode, 0)
 
+    def test_accepts_tracked_source_with_option_like_name(self):
+        lcov = b"TN:\nSF:-tracked.ts\nDA:1,1\nend_of_record\n"
+        self.assertEqual(self.run_validator(self.archive(lcov=lcov)).returncode, 0)
+
+    def test_accepts_valid_push_revision_boundary(self):
+        manifest = self.manifest()
+        manifest["producer"]["event"] = "push"
+        manifest["candidate"]["sha"] = self.base
+        manifest["candidate"]["pull_request"] = 0
+        result = self.run_validator(
+            self.archive(manifest_bytes=json.dumps(manifest).encode()),
+            candidate_sha=self.base,
+            event="push",
+            pull_request=0,
+        )
+        self.assertEqual(result.returncode, 0, result.stderr)
+
+    def test_rejects_option_like_traversal_and_control_git_revisions(self):
+        for field, value in (
+            ("sha", "--help"),
+            ("base_sha", "../HEAD"),
+            ("sha", "1" * 39 + "\n"),
+        ):
+            with self.subTest(field=field, value=value):
+                manifest = self.manifest()
+                manifest["candidate"][field] = value
+                result = self.run_validator(
+                    self.archive(manifest_bytes=json.dumps(manifest).encode()),
+                    candidate_sha=value if field == "sha" else SHA,
+                    base_sha=value if field == "base_sha" else self.base,
+                )
+                self.assertNotEqual(result.returncode, 0)
+                self.assertIn("must be a lowercase 40-character Git SHA", result.stderr)
+
     def test_rejects_traversal_and_url_sources(self):
         for source in ("../src/a.ts", "/src/a.ts", "https://evil.invalid/a.ts", "src//a.ts"):
             with self.subTest(source=source):

scripts/ci/test_validate_workflow_run.cjs

@@ -68,7 +68,7 @@ const server = createServer((request, response) => {
   response.end(JSON.stringify(fixture || { message: 'not found' }));
 });
 
-server.listen(0, '127.0.0.1', () => {
+function runValidator(envOverrides = {}) {
   const child = spawn(process.execPath, [join(__dirname, 'validate-workflow-run.cjs')], {
     env: {
       ...process.env,
@@ -82,50 +82,65 @@ server.listen(0, '127.0.0.1', () => {
       TRIGGER_RUN_ATTEMPT: '1',
       TRIGGER_HEAD_REPOSITORY_ID: '99',
       TRIGGER_HEAD_SHA: head,
+      ...envOverrides,
     },
     stdio: ['ignore', 'pipe', 'pipe'],
   });
   let stderr = '';
   child.stderr.on('data', (chunk) => { stderr += chunk; });
-  child.on('close', (code) => {
-    assert.equal(code, 0, stderr);
-    const output = readFileSync(outputPath, 'utf8');
-    assert.match(output, new RegExp(`candidate_sha=${head}`));
-    assert.match(output, /workflow_ref=RandomCodeSpace\/kb\/\.github\/workflows\/quality\.yml@refs\/pull\/7\/merge/);
-    assert.match(output, new RegExp(`workflow_sha=${merge}`));
-    assert.match(output, /pull_request=7/);
-    assert.match(output, new RegExp(`base_sha=${base}`));
-    assert.match(output, /candidate_repository=ExampleContributor\/kb/);
-    const requestCount = requests.length;
-    event.workflow_run.id = runId + 1;
-    writeFileSync(eventPath, JSON.stringify(event));
-    const hostile = spawn(process.execPath, [join(__dirname, 'validate-workflow-run.cjs')], {
-      env: {
-        ...process.env,
-        GITHUB_API_URL: `http://127.0.0.1:${server.address().port}`,
-        GITHUB_EVENT_PATH: eventPath,
-        GITHUB_OUTPUT: outputPath,
-        GITHUB_REPOSITORY: repository,
-        GITHUB_TOKEN: 'test-token',
-        TRIGGER_RUN_ID: String(runId),
-        TRIGGER_WORKFLOW_ID: '5',
-        TRIGGER_RUN_ATTEMPT: '1',
-        TRIGGER_HEAD_REPOSITORY_ID: '99',
-        TRIGGER_HEAD_SHA: head,
-      },
-      stdio: ['ignore', 'pipe', 'pipe'],
-    });
-    let hostileStderr = '';
-    hostile.stderr.on('data', (chunk) => { hostileStderr += chunk; });
-    hostile.on('close', async (hostileCode) => {
-      assert.notEqual(hostileCode, 0);
-      assert.match(hostileStderr, /trusted workflow run id mismatch/);
-      assert.equal(requests.length, requestCount, 'mismatched event id must not reach the network');
-      await testManifestDescriptorReads();
-      await testSonarOriginPinning();
-      server.close(() => console.log('CI JavaScript hostile fixtures passed'));
-    });
-  });
+  return new Promise((resolve) => child.on('close', (code) => resolve({ code, stderr })));
+}
+
+async function runWorkflowTests() {
+  const valid = await runValidator();
+  assert.equal(valid.code, 0, valid.stderr);
+  const output = readFileSync(outputPath, 'utf8');
+  assert.match(output, new RegExp(`candidate_sha=${head}`));
+  assert.match(output, /workflow_ref=RandomCodeSpace\/kb\/\.github\/workflows\/quality\.yml@refs\/pull\/7\/merge/);
+  assert.match(output, new RegExp(`workflow_sha=${merge}`));
+  assert.match(output, /pull_request=7/);
+  assert.match(output, new RegExp(`base_sha=${base}`));
+  assert.match(output, /candidate_repository=ExampleContributor\/kb/);
+
+  const requestCount = requests.length;
+  const hostileEnvironmentCases = [
+    [{ TRIGGER_RUN_ID: `${runId}/../admin` }, /TRIGGER_RUN_ID must be a positive integer/],
+    [{ TRIGGER_RUN_ID: '9007199254740992' }, /TRIGGER_RUN_ID must be a positive safe integer/],
+    [{ GITHUB_REPOSITORY: 'RandomCodeSpace/kb/../admin' }, /GITHUB_REPOSITORY is invalid/],
+    [{ GITHUB_API_URL: `http://127.0.0.1:${server.address().port}//` }, /GITHUB_API_URL is invalid/],
+  ];
+  for (const [envOverrides, expectedError] of hostileEnvironmentCases) {
+    const hostile = await runValidator(envOverrides);
+    assert.notEqual(hostile.code, 0);
+    assert.match(hostile.stderr, expectedError);
+    assert.equal(requests.length, requestCount, 'invalid API path identifier must not reach the network');
+  }
+
+  event.workflow_run.head_repository.full_name = 'ExampleContributor/kb/../../admin';
+  writeFileSync(eventPath, JSON.stringify(event));
+  const injectedRepository = await runValidator();
+  assert.notEqual(injectedRepository.code, 0);
+  assert.match(injectedRepository.stderr, /candidate repository is invalid/);
+  assert.equal(requests.length, requestCount, 'injected repository path must not reach the network');
+
+  event.workflow_run.head_repository.full_name = candidateRepository;
+  event.workflow_run.id = runId + 1;
+  writeFileSync(eventPath, JSON.stringify(event));
+  const mismatchedRun = await runValidator();
+  assert.notEqual(mismatchedRun.code, 0);
+  assert.match(mismatchedRun.stderr, /trusted workflow run id mismatch/);
+  assert.equal(requests.length, requestCount, 'mismatched event id must not reach the network');
+}
+
+server.listen(0, '127.0.0.1', () => {
+  runWorkflowTests()
+    .then(testManifestDescriptorReads)
+    .then(testSonarOriginPinning)
+    .then(() => server.close(() => console.log('CI JavaScript hostile fixtures passed')))
+    .catch((error) => server.close(() => {
+      console.error(error);
+      process.exitCode = 1;
+    }));
 });
 
 async function testManifestDescriptorReads() {

scripts/ci/validate-coverage-artifact.py

@@ -255,14 +255,14 @@ def unique_object(pairs):
     manifest_base = manifest["candidate"]["base_sha"]
     if manifest_base != empty_tree:
         base_exists = subprocess.run(
-            ["git", "-C", str(args.repository_root), "cat-file", "-e", f"{manifest_base}^{{commit}}"],
+            ["git", "-C", str(args.repository_root), "cat-file", "-e", "--end-of-options", f"{manifest_base}^{{commit}}"],
             stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, check=False,
         )
         if base_exists.returncode != 0:
             fail("candidate event base is not an available commit")
         if args.event != "pull_request":
             ancestor = subprocess.run(
-                ["git", "-C", str(args.repository_root), "merge-base", "--is-ancestor", manifest_base, args.candidate_sha],
+                ["git", "-C", str(args.repository_root), "merge-base", "--is-ancestor", "--end-of-options", manifest_base, args.candidate_sha],
                 stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, check=False,
             )
             if ancestor.returncode != 0:

scripts/ci/validate-workflow-run.cjs

@@ -37,9 +37,25 @@
   return value;
 }
 
-async function github(path) {
-  const apiUrl = process.env.GITHUB_API_URL || 'https://api.github.com';
-  const response = await fetch(`${apiUrl}${path}`, {
+function githubPathIdentifier(value, label) {
+  if (typeof value === 'number') return String(safeInteger(value, label));
+  if (typeof value !== 'string' || !value || value.length > 100 || !/^[A-Za-z0-9_.-]+$/.test(value)) {
+    throw new Error(`${label} is invalid`);
+  }
+  return encodeURIComponent(value);
+}
+
+async function github(segments, query = {}) {
+  if (!Array.isArray(segments) || !segments.length) throw new Error('GitHub API path is invalid');
+  const apiUrl = new URL(process.env.GITHUB_API_URL || 'https://api.github.com');
+  if (!['http:', 'https:'].includes(apiUrl.protocol) || apiUrl.username || apiUrl.password || apiUrl.search || apiUrl.hash || apiUrl.pathname.includes('//')) {
+    throw new Error('GITHUB_API_URL is invalid');
+  }
+  const path = segments.map((segment, index) => githubPathIdentifier(segment, `GitHub API path segment ${index + 1}`)).join('/');
+  const basePath = apiUrl.pathname === '/' ? '' : apiUrl.pathname.endsWith('/') ? apiUrl.pathname.slice(0, -1) : apiUrl.pathname;
+  apiUrl.pathname = `${basePath}/${path}`;
+  for (const [key, value] of Object.entries(query)) apiUrl.searchParams.set(key, String(value));
+  const response = await fetch(apiUrl, {
     headers: {
       Accept: 'application/vnd.github+json',
       Authorization: `Bearer ${required('GITHUB_TOKEN')}`,
@@ -60,7 +76,7 @@
 
 async function main() {
   const event = JSON.parse(readFileSync(required('GITHUB_EVENT_PATH'), 'utf8'));
-  const repository = required('GITHUB_REPOSITORY');
+  const repository = repositoryName(required('GITHUB_REPOSITORY'), 'GITHUB_REPOSITORY');
   const triggerRunId = environmentInteger('TRIGGER_RUN_ID');
   const triggerWorkflowId = environmentInteger('TRIGGER_WORKFLOW_ID');
   const triggerRunAttempt = environmentInteger('TRIGGER_RUN_ATTEMPT');
@@ -83,8 +99,8 @@
   equal(safeInteger(trigger.head_repository.id, 'head repository id'), triggerHeadRepositoryId, 'trusted head repository id');
   equal(trigger.head_sha, triggerHeadSha, 'trusted head SHA');
 
-  const encodedRepo = repository.split('/').map(encodeURIComponent).join('/');
-  const run = await github(`/repos/${encodedRepo}/actions/runs/${triggerRunId}`);
+  const repositorySegments = repository.split('/');
+  const run = await github(['repos', ...repositorySegments, 'actions', 'runs', triggerRunId]);
   equal(run.id, triggerRunId, 'API run id');
   equal(run.workflow_id, triggerWorkflowId, 'API workflow id');
   equal(run.name, WORKFLOW_NAME, 'API workflow name');
@@ -96,12 +112,12 @@
   equal(run.head_sha, triggerHeadSha, 'API head SHA');
   equal(run.run_attempt, triggerRunAttempt, 'API run attempt');
   const workflowFile = WORKFLOW_PATH.split('/').at(-1);
-  const workflow = await github(`/repos/${encodedRepo}/actions/workflows/${encodeURIComponent(workflowFile)}`);
+  const workflow = await github(['repos', ...repositorySegments, 'actions', 'workflows', workflowFile]);
   equal(workflow.id, triggerWorkflowId, 'workflow-by-path id');
   equal(workflow.path, WORKFLOW_PATH, 'workflow-by-path path');
   equal(workflow.name, WORKFLOW_NAME, 'workflow-by-path name');
 
-  const jobs = await github(`/repos/${encodedRepo}/actions/runs/${triggerRunId}/jobs?filter=latest&per_page=100`);
+  const jobs = await github(['repos', ...repositorySegments, 'actions', 'runs', triggerRunId, 'jobs'], { filter: 'latest', per_page: 100 });
   const jobMatches = jobs.jobs.filter((job) => job.name === JOB_NAME && job.run_attempt === triggerRunAttempt);
   if (jobMatches.length !== 1) throw new Error(`expected exactly one ${JOB_NAME} job, found ${jobMatches.length}`);
   const job = jobMatches[0];
@@ -111,7 +127,7 @@
   equal(job.conclusion, 'success', 'producer job conclusion');
   equal(job.head_sha, triggerHeadSha, 'producer job head SHA');
 
-  const artifacts = await github(`/repos/${encodedRepo}/actions/runs/${triggerRunId}/artifacts?per_page=100`);
+  const artifacts = await github(['repos', ...repositorySegments, 'actions', 'runs', triggerRunId, 'artifacts'], { per_page: 100 });
   equal(artifacts.total_count, 1, 'artifact count');
   equal(artifacts.artifacts.length, 1, 'returned artifact count');
   const artifact = artifacts.artifacts[0];
@@ -125,8 +141,7 @@
   equal(artifact.workflow_run?.head_sha, triggerHeadSha, 'artifact head SHA');
   equal(artifact.workflow_run?.head_repository_id, triggerHeadRepositoryId, 'artifact head repository id');
 
-  const encodedCandidateRepo = candidateRepository.split('/').map(encodeURIComponent).join('/');
-  const commit = await github(`/repos/${encodedCandidateRepo}/git/commits/${triggerHeadSha}`);
+  const commit = await github(['repos', ...candidateRepository.split('/'), 'git', 'commits', triggerHeadSha]);
   equal(commit.sha, triggerHeadSha, 'candidate commit SHA');
   const candidateTree = commit.tree?.sha;
   if (!/^[0-9a-f]{40}$/.test(candidateTree || '')) throw new Error('candidate tree is invalid');
@@ -149,7 +164,7 @@
     baseSha = pull.base?.sha;
     baseRef = pull.base?.ref;
     candidateRef = pull.head?.ref;
-    const pullDetails = await github(`/repos/${encodedRepo}/pulls/${pull.number}`);
+    const pullDetails = await github(['repos', ...repositorySegments, 'pulls', pull.number]);
     equal(pullDetails.head?.sha, triggerHeadSha, 'pull request API head SHA');
     equal(pullDetails.head?.repo?.full_name, candidateRepository, 'pull request API head repository');
     equal(pullDetails.base?.repo?.full_name, repository, 'pull request API base repository');