From 5a5cf5a8abdff4ccad9506f53eade02841a82c51 Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Sat, 1 Aug 2026 14:07:07 +0000 Subject: [PATCH 1/2] fix(security): remediate highest-risk sonar findings --- scripts/ci/test_validate_coverage_artifact.py | 48 +++- scripts/ci/test_validate_workflow_run.cjs | 94 ++++---- scripts/ci/validate-coverage-artifact.py | 4 +- scripts/ci/validate-workflow-run.cjs | 38 +++- src/lib/outbox.test.ts | 210 ++++++++++++++++++ src/lib/outbox.ts | 115 +++++++++- src/lib/remote.test.ts | 25 +++ src/lib/remote.ts | 6 +- 8 files changed, 468 insertions(+), 72 deletions(-) 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..15c2cf4 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,64 @@ 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/], + ]; + 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..86542ff 100644 --- a/scripts/ci/validate-workflow-run.cjs +++ b/scripts/ci/validate-workflow-run.cjs @@ -37,9 +37,24 @@ 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) { + throw new Error('GITHUB_API_URL is invalid'); + } + const path = segments.map((segment, index) => githubPathIdentifier(segment, `GitHub API path segment ${index + 1}`)).join('/'); + apiUrl.pathname = `${apiUrl.pathname.replace(/\/+$/, '')}/${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 +75,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 +98,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 +111,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 +126,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 +140,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 +163,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'); diff --git a/src/lib/outbox.test.ts b/src/lib/outbox.test.ts index c7f88b1..996e72d 100644 --- a/src/lib/outbox.test.ts +++ b/src/lib/outbox.test.ts @@ -228,6 +228,134 @@ describe('MetadataOutbox', () => { expect(JSON.stringify([...storage.values])).not.toContain('secret'); }); + it('serializes only validated import fields and never invokes caller serialization hooks', async () => { + const toJSON = vi.fn(() => ({ poisoned: true })); + const item = { + ...importRequest.items[0], + ignored: 'attacker-controlled extra field', + toJSON, + }; + const outbox = new MetadataOutbox('alice', { + storage, locks: locks as unknown as LockManager, generation: generations(), + }); + + await outbox.enqueueImportLinks({ source: importRequest.source, items: [item] }); + + expect(toJSON).not.toHaveBeenCalled(); + expect(outbox.records()).toEqual([{ + version: 1, + generation: 'generation-1', + kind: 'import', + state: 'queued', + source: importRequest.source, + item: importRequest.items[0], + }]); + expect(JSON.stringify([...storage.values])).not.toContain('ignored'); + expect(JSON.stringify([...storage.values])).not.toContain('poisoned'); + }); + + it.each([ + ['non-object request', null], + ['blank source', { ...importRequest, source: ' ' }], + ['non-array items', { ...importRequest, items: {} }], + ['too many items', { ...importRequest, items: Array(101).fill(importRequest.items[0]) }], + ['non-object item', { ...importRequest, items: [null] }], + ['blank field', { + ...importRequest, + items: [{ ...importRequest.items[0], link: ' ' }], + }], + ['line break', { + ...importRequest, + items: [{ ...importRequest.items[0], title: 'forged\nmetadata' }], + }], + ['carriage return', { + ...importRequest, + items: [{ ...importRequest.items[0], link: 'forged\rmetadata' }], + }], + ['oversized external key', { + ...importRequest, + items: [{ ...importRequest.items[0], external_key: 'é'.repeat(1025) }], + }], + ['oversized URL', { + ...importRequest, + items: [{ ...importRequest.items[0], url: 'é'.repeat(1025) }], + }], + ['oversized title', { + ...importRequest, + items: [{ ...importRequest.items[0], title: 'é'.repeat(251) }], + }], + ['non-string field', { + ...importRequest, + items: [{ ...importRequest.items[0], url: 7 }], + }], + ])('rejects invalid import metadata atomically: %s', async (_label, request) => { + const outbox = new MetadataOutbox('alice', { + storage, locks: locks as unknown as LockManager, generation: generations(), + }); + + await expect(outbox.enqueueImportLinks( + request as unknown as Parameters[0], + )).rejects.toThrow('invalid outbox'); + expect(storage.values.size).toBe(0); + }); + + it('sanitizes server-controlled error metadata before persisting it', async () => { + const malicious = `invalid\r\n${'x'.repeat(300)}`; + const statuses = vi.fn(); + const outbox = new MetadataOutbox('alice', { + storage, + locks: locks as unknown as LockManager, + generation: generations(), + sendImport: () => Promise.reject(Object.assign(new Error(malicious), { status: 400 })), + onStatus: statuses, + }); + + await outbox.enqueueImportLinks(importRequest); + await outbox.drain(identity); + + expect(outbox.records()[0]).toMatchObject({ + state: 'blocked', + error: expect.not.stringContaining('\n'), + }); + expect(outbox.records()[0]?.error).toHaveLength(200); + expect(statuses).toHaveBeenCalledWith({ kind: 'blocked', message: malicious }); + }); + + it('omits empty or non-string error metadata from durable records', async () => { + const emptyError = new MetadataOutbox('alice', { + storage, + locks: locks as unknown as LockManager, + generation: generations(), + sendImport: () => Promise.reject(Object.assign(new Error(''), { status: 400 })), + }); + await emptyError.enqueueImportLinks(importRequest); + await emptyError.drain(identity); + expect(emptyError.records()[0]).not.toHaveProperty('error'); + + const key = [...storage.values.keys()][0]!; + storage.setItem(key, JSON.stringify({ + ...JSON.parse(storage.getItem(key)!), + state: 'sending', + error: 7, + })); + await emptyError.reconcile(cancelled, new Map()); + expect(emptyError.records()[0]).toMatchObject({ state: 'retry' }); + expect(emptyError.records()[0]).not.toHaveProperty('error'); + }); + + it.each([ + ['blank task id', ' ', 'valid reason'], + ['line break in reason', 'client-1', 'forged\nmetadata'], + ['oversized reason', 'client-1', 'é'.repeat(1001)], + ])('rejects invalid tombstone metadata before storage: %s', async (_label, taskId, reason) => { + const outbox = new MetadataOutbox('alice', { + storage, locks: locks as unknown as LockManager, generation: generations(), + }); + + await expect(outbox.enqueueTombstone(taskId, reason)).rejects.toThrow('invalid outbox'); + expect(storage.values.size).toBe(0); + }); + it.each([ ['2xx', undefined], ['network', new TypeError('offline')], @@ -366,6 +494,88 @@ describe('MetadataOutbox', () => { ); }); + it('canonicalizes legacy records instead of copying attacker-controlled fields', () => { + const logicalKey = `import:${importRequest.items[0]!.external_key}`; + const legacyKey = `kb.outbox.v1.alice.${encodeURIComponent(logicalKey)}`; + storage.values.set(legacyKey, JSON.stringify({ + version: 1, + generation: 'legacy-generation', + kind: 'import', + state: 'queued', + source: importRequest.source, + item: { + ...importRequest.items[0], + ignored: 'attacker-controlled extra field', + toJSON: { poisoned: true }, + }, + ignored: 'attacker-controlled record field', + })); + + const outbox = new MetadataOutbox('alice', { + storage, locks: locks as unknown as LockManager, generation: generations(), + }); + + expect(outbox.records()).toEqual([{ + version: 1, + generation: 'legacy-generation', + kind: 'import', + state: 'queued', + source: importRequest.source, + item: importRequest.items[0], + }]); + const migrated = [...storage.values.entries()].find(([key]) => key !== legacyKey); + expect(migrated).toBeDefined(); + expect(migrated![1]).not.toContain('ignored'); + expect(migrated![1]).not.toContain('toJSON'); + expect(migrated![1]).not.toContain('poisoned'); + }); + + it.each([ + ['line break', { ...importRequest.items[0], title: 'forged\nmetadata' }], + ['oversized URL', { ...importRequest.items[0], url: 'é'.repeat(1025) }], + ])('retains but does not migrate invalid legacy metadata: %s', (_label, item) => { + const logicalKey = `import:${item.external_key}`; + const legacyKey = `kb.outbox.v1.alice.${encodeURIComponent(logicalKey)}`; + const raw = JSON.stringify({ + version: 1, + generation: 'legacy-generation', + kind: 'import', + state: 'queued', + source: importRequest.source, + item, + }); + storage.values.set(legacyKey, raw); + + const outbox = new MetadataOutbox('alice', { + storage, locks: locks as unknown as LockManager, generation: generations(), + }); + + expect(outbox.records()).toEqual([]); + expect(storage.values).toEqual(new Map([[legacyKey, raw]])); + }); + + it('retains a valid legacy record when canonical migration storage fails', () => { + const logicalKey = `import:${importRequest.items[0]!.external_key}`; + const legacyKey = `kb.outbox.v1.alice.${encodeURIComponent(logicalKey)}`; + const raw = JSON.stringify({ + version: 1, + generation: 'legacy-generation', + kind: 'import', + state: 'queued', + source: importRequest.source, + item: importRequest.items[0], + }); + storage.values.set(legacyKey, raw); + storage.failSet = true; + + const outbox = new MetadataOutbox('alice', { + storage, locks: locks as unknown as LockManager, generation: generations(), + }); + + expect(outbox.records()).toEqual([]); + expect(storage.values).toEqual(new Map([[legacyKey, raw]])); + }); + it('does not lose interleaved two-instance additions and removals', async () => { const first = new MetadataOutbox('alice', { storage, locks: locks as unknown as LockManager, generation: generations(), diff --git a/src/lib/outbox.ts b/src/lib/outbox.ts index b5c081a..643a74d 100644 --- a/src/lib/outbox.ts +++ b/src/lib/outbox.ts @@ -11,6 +11,11 @@ import { const PREFIX = 'kb.outbox.v1'; const LOCK_PREFIX = 'kb:outbox:'; +const MAX_IMPORT_ITEMS = 100; +const MAX_EXTERNAL_KEY_BYTES = 2048; +const MAX_URL_BYTES = 2048; +const MAX_TITLE_BYTES = 500; +const MAX_REASON_BYTES = 2000; type OutboxState = 'awaiting_canonical' | 'queued' | 'sending' | 'retry' | 'blocked'; @@ -152,6 +157,91 @@ function isRecord(value: unknown): value is Record { return typeof value === 'object' && value !== null && !Array.isArray(value); } +function validatedText( + value: unknown, + field: string, + maxBytes?: number, +): string { + if ( + typeof value !== 'string' || + value.trim() === '' || + value.includes('\r') || + value.includes('\n') || + (maxBytes !== undefined && new TextEncoder().encode(value).byteLength > maxBytes) + ) { + throw new TypeError(`invalid outbox ${field}`); + } + return value; +} + +function validatedImportRequest(req: RecordImportLinksRequest): RecordImportLinksRequest { + if (!isRecord(req)) throw new TypeError('invalid outbox import request'); + const source = validatedText(req.source, 'import source'); + if (!Array.isArray(req.items) || req.items.length > MAX_IMPORT_ITEMS) { + throw new TypeError('invalid outbox import items'); + } + const items = req.items.map((item) => { + if (!isRecord(item)) throw new TypeError('invalid outbox import item'); + return { + external_key: validatedText( + item.external_key, + 'import external key', + MAX_EXTERNAL_KEY_BYTES, + ), + link: validatedText(item.link, 'import link'), + url: validatedText(item.url, 'import URL', MAX_URL_BYTES), + title: validatedText(item.title, 'import title', MAX_TITLE_BYTES), + }; + }); + return { source, items }; +} + +function storedError(value: unknown): string | undefined { + if (typeof value !== 'string') return undefined; + const safe = value.replace(/[\r\n]+/g, ' ').slice(0, 200); + return safe === '' ? undefined : safe; +} + +function validatedStorageRecord(record: OutboxRecord): OutboxRecord { + const generation = validatedText(record.generation, 'generation'); + const error = storedError(record.error); + if (record.kind === 'import') { + const validated = validatedImportRequest({ source: record.source, items: [record.item] }); + return { + version: 1, + generation, + kind: 'import', + state: record.state, + source: validated.source, + item: validated.items[0]!, + ...(error === undefined ? {} : { error }), + }; + } + const clientTaskId = validatedText(record.clientTaskId, 'tombstone task ID'); + const reason = validatedText(record.reason, 'tombstone reason', MAX_REASON_BYTES); + if (record.state === 'awaiting_canonical') { + return { + version: 1, + generation, + kind: 'tombstone', + state: 'awaiting_canonical', + clientTaskId, + reason, + ...(error === undefined ? {} : { error }), + }; + } + return { + version: 1, + generation, + kind: 'tombstone', + state: record.state, + clientTaskId, + canonicalTaskId: validatedText(record.canonicalTaskId, 'canonical task ID'), + reason, + ...(error === undefined ? {} : { error }), + }; +} + function parseRecord(raw: string | null): OutboxRecord | null { if (raw === null) return null; try { @@ -244,8 +334,12 @@ export class MetadataOutbox { if (key !== expected) continue; const target = recordKey(this.ns, logicalKey(record)); if (this.storage.getItem(target) === null) { - const raw = this.storage.getItem(key); - if (raw !== null) this.storage.setItem(target, raw); + try { + this.write(target, record); + } catch (error) { + if (error instanceof TypeError) continue; + throw error; + } } } } catch { @@ -296,7 +390,7 @@ export class MetadataOutbox { } private write(key: string, record: OutboxRecord): void { - this.storage.setItem(key, JSON.stringify(record)); + this.storage.setItem(key, JSON.stringify(validatedStorageRecord(record))); } private fresh(record: NewOutboxRecord): OutboxRecord { @@ -309,12 +403,14 @@ export class MetadataOutbox { /** Persist the user's reason before the board PUT can acknowledge an ID. */ async enqueueTombstone(clientTaskId: string, reason: string): Promise { - const key = recordKey(this.ns, tombstoneLogicalKey(clientTaskId)); + const safeClientTaskId = validatedText(clientTaskId, 'tombstone task ID'); + const safeReason = validatedText(reason, 'tombstone reason', MAX_REASON_BYTES); + const key = recordKey(this.ns, tombstoneLogicalKey(safeClientTaskId)); const record = this.fresh({ kind: 'tombstone', state: 'awaiting_canonical', - clientTaskId, - reason, + clientTaskId: safeClientTaskId, + reason: safeReason, }); const written = await this.locked(() => this.write(key, record)); if (written === undefined && !this.locks?.request) { @@ -326,10 +422,13 @@ export class MetadataOutbox { } async enqueueImportLinks(req: RecordImportLinksRequest): Promise { + const validated = validatedImportRequest(req); const writeAll = () => { - for (const item of req.items) { + for (const item of validated.items) { const key = recordKey(this.ns, importLogicalKey(item.external_key)); - this.write(key, this.fresh({ kind: 'import', state: 'queued', source: req.source, item })); + this.write(key, this.fresh({ + kind: 'import', state: 'queued', source: validated.source, item, + })); } }; const written = await this.locked(writeAll); diff --git a/src/lib/remote.test.ts b/src/lib/remote.test.ts index f382396..6a99b9a 100644 --- a/src/lib/remote.test.ts +++ b/src/lib/remote.test.ts @@ -2463,6 +2463,31 @@ describe('RemoteStore concurrency', () => { })).toBe(false); }); + it.each([ + ['mixed-case', ['server-A', 'server-a', 'Server-b']], + ['numeric-like', ['server-2', 'server-10', 'server-01']], + ['non-ASCII', ['server-ä', 'server-Ω', 'server-😀']], + ])('compares %s deleted canonical IDs independently of insertion order', (_kind, ids) => { + const value = board('unchanged'); + const canonicalTaskIDs = new Map([[value.tasks[0]!.id, 'server-live']]); + const base = { + board: value, + canonicalTaskIDs, + deletedCanonicalIDs: new Set(ids), + migratedRaw: false, + pendingBoardWrite: null, + }; + + expect(sameBoardSemantics(base, { + ...base, + deletedCanonicalIDs: new Set([...ids].reverse()), + })).toBe(true); + expect(sameBoardSemantics(base, { + ...base, + deletedCanonicalIDs: new Set([...ids.slice(0, -1), `${ids.at(-1)}-different`]), + })).toBe(false); + }); + it('exposes a current-epoch guard across an awaited success callback', async () => { const store = new RemoteStore(); const release = deferred(); diff --git a/src/lib/remote.ts b/src/lib/remote.ts index 62fa1ff..d3dbda1 100644 --- a/src/lib/remote.ts +++ b/src/lib/remote.ts @@ -195,10 +195,8 @@ export function sameBoardSemantics( canonicalSequence(current.board, current.canonicalTaskIDs), canonicalSequence(target.board, target.canonicalTaskIDs), ) && - same( - [...current.deletedCanonicalIDs].sort(), - [...target.deletedCanonicalIDs].sort(), - ) && + current.deletedCanonicalIDs.size === target.deletedCanonicalIDs.size && + [...current.deletedCanonicalIDs].every((id) => target.deletedCanonicalIDs.has(id)) && current.migratedRaw === target.migratedRaw && same(current.pendingBoardWrite, target.pendingBoardWrite) ); From fc6a48cd47eb17632eca4fe4f57c01c9b033294d Mon Sep 17 00:00:00 2001 From: Amit Kumar Date: Sat, 1 Aug 2026 14:15:02 +0000 Subject: [PATCH 2/2] fix(ci): separate control-plane hardening --- scripts/ci/test_validate_coverage_artifact.py | 48 ++-------- scripts/ci/test_validate_workflow_run.cjs | 94 ++++++++----------- scripts/ci/validate-coverage-artifact.py | 4 +- scripts/ci/validate-workflow-run.cjs | 38 +++----- 4 files changed, 60 insertions(+), 124 deletions(-) diff --git a/scripts/ci/test_validate_coverage_artifact.py b/scripts/ci/test_validate_coverage_artifact.py index 4a460c1..e162cd6 100644 --- a/scripts/ci/test_validate_coverage_artifact.py +++ b/scripts/ci/test_validate_coverage_artifact.py @@ -24,13 +24,12 @@ 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", "-tracked.ts", "internal/a.go", "go.mod"], 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, "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" @@ -85,18 +84,17 @@ 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, *, candidate_sha=SHA, base_sha=None, event="pull_request", pull_request=7): + def run_validator(self, archive, output=None): 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", 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), + "--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), "--maintenance", "false", ] return subprocess.run(args, text=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE) @@ -104,40 +102,6 @@ def run_validator(self, archive, output=None, *, candidate_sha=SHA, base_sha=Non 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 15c2cf4..5c02609 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' })); }); -function runValidator(envOverrides = {}) { +server.listen(0, '127.0.0.1', () => { const child = spawn(process.execPath, [join(__dirname, 'validate-workflow-run.cjs')], { env: { ...process.env, @@ -82,64 +82,50 @@ function runValidator(envOverrides = {}) { 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; }); - 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/], - ]; - 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; - })); + 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')); + }); + }); }); async function testManifestDescriptorReads() { diff --git a/scripts/ci/validate-coverage-artifact.py b/scripts/ci/validate-coverage-artifact.py index f4af115..f0b080f 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", "--end-of-options", f"{manifest_base}^{{commit}}"], + ["git", "-C", str(args.repository_root), "cat-file", "-e", 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", "--end-of-options", manifest_base, args.candidate_sha], + ["git", "-C", str(args.repository_root), "merge-base", "--is-ancestor", 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 86542ff..e4c0cae 100644 --- a/scripts/ci/validate-workflow-run.cjs +++ b/scripts/ci/validate-workflow-run.cjs @@ -37,24 +37,9 @@ function repositoryName(value, label) { return value; } -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) { - throw new Error('GITHUB_API_URL is invalid'); - } - const path = segments.map((segment, index) => githubPathIdentifier(segment, `GitHub API path segment ${index + 1}`)).join('/'); - apiUrl.pathname = `${apiUrl.pathname.replace(/\/+$/, '')}/${path}`; - for (const [key, value] of Object.entries(query)) apiUrl.searchParams.set(key, String(value)); - const response = await fetch(apiUrl, { +async function github(path) { + const apiUrl = process.env.GITHUB_API_URL || 'https://api.github.com'; + const response = await fetch(`${apiUrl}${path}`, { headers: { Accept: 'application/vnd.github+json', Authorization: `Bearer ${required('GITHUB_TOKEN')}`, @@ -75,7 +60,7 @@ function output(values) { async function main() { const event = JSON.parse(readFileSync(required('GITHUB_EVENT_PATH'), 'utf8')); - const repository = repositoryName(required('GITHUB_REPOSITORY'), 'GITHUB_REPOSITORY'); + const repository = required('GITHUB_REPOSITORY'); const triggerRunId = environmentInteger('TRIGGER_RUN_ID'); const triggerWorkflowId = environmentInteger('TRIGGER_WORKFLOW_ID'); const triggerRunAttempt = environmentInteger('TRIGGER_RUN_ATTEMPT'); @@ -98,8 +83,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 repositorySegments = repository.split('/'); - const run = await github(['repos', ...repositorySegments, 'actions', 'runs', triggerRunId]); + const encodedRepo = repository.split('/').map(encodeURIComponent).join('/'); + const run = await github(`/repos/${encodedRepo}/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'); @@ -111,12 +96,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', ...repositorySegments, 'actions', 'workflows', workflowFile]); + const workflow = await github(`/repos/${encodedRepo}/actions/workflows/${encodeURIComponent(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', ...repositorySegments, 'actions', 'runs', triggerRunId, 'jobs'], { filter: 'latest', per_page: 100 }); + const jobs = await github(`/repos/${encodedRepo}/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]; @@ -126,7 +111,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', ...repositorySegments, 'actions', 'runs', triggerRunId, 'artifacts'], { per_page: 100 }); + const artifacts = await github(`/repos/${encodedRepo}/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]; @@ -140,7 +125,8 @@ 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 commit = await github(['repos', ...candidateRepository.split('/'), 'git', 'commits', triggerHeadSha]); + const encodedCandidateRepo = candidateRepository.split('/').map(encodeURIComponent).join('/'); + const commit = await github(`/repos/${encodedCandidateRepo}/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'); @@ -163,7 +149,7 @@ async function main() { baseSha = pull.base?.sha; baseRef = pull.base?.ref; candidateRef = pull.head?.ref; - const pullDetails = await github(['repos', ...repositorySegments, 'pulls', pull.number]); + const pullDetails = await github(`/repos/${encodedRepo}/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');