diff --git a/scripts/ci/test_validate_coverage_artifact.py b/scripts/ci/test_validate_coverage_artifact.py index e162cd6..4a460c1 100644 --- a/scripts/ci/test_validate_coverage_artifact.py +++ b/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", "ci@example.invalid"], 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,17 +85,18 @@ 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) @@ -102,6 +104,40 @@ def run_validator(self, archive, output=None): 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): diff --git a/scripts/ci/test_validate_workflow_run.cjs b/scripts/ci/test_validate_workflow_run.cjs index 5c02609..affbf39 100644 --- a/scripts/ci/test_validate_workflow_run.cjs +++ b/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() { diff --git a/scripts/ci/validate-coverage-artifact.py b/scripts/ci/validate-coverage-artifact.py index f0b080f..f4af115 100644 --- a/scripts/ci/validate-coverage-artifact.py +++ b/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: diff --git a/scripts/ci/validate-workflow-run.cjs b/scripts/ci/validate-workflow-run.cjs index e4c0cae..55252b4 100644 --- a/scripts/ci/validate-workflow-run.cjs +++ b/scripts/ci/validate-workflow-run.cjs @@ -37,9 +37,25 @@ function repositoryName(value, label) { 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 @@ function output(values) { 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 @@ async function main() { 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 @@ async function main() { 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 @@ async function main() { 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 @@ async function main() { 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 @@ async function main() { 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');