From 0ce4e4285610ef9201f92bfa97eb6fec773fd8e7 Mon Sep 17 00:00:00 2001 From: garrytan Date: Mon, 19 Oct 2026 18:26:05 +0000 Subject: [PATCH] fix(cso): security expectations may list several safe statuses while excluding the vulnerable marker (cherry picked from commit aa9f45e0ece0e12152e7d02bf88e1559ee42b1eb) --- lib/cso/contracts.ts | 40 ++++++++++++++++++++++++++++---------- lib/cso/verifier.ts | 2 +- test/cso-contracts.test.ts | 11 +++++++++++ test/cso-verifier.test.ts | 13 +++++++++++++ 4 files changed, 55 insertions(+), 11 deletions(-) diff --git a/lib/cso/contracts.ts b/lib/cso/contracts.ts index 1f06b4565..c1bc250af 100644 --- a/lib/cso/contracts.ts +++ b/lib/cso/contracts.ts @@ -180,7 +180,7 @@ export interface HttpAssertion { method: 'GET' | 'POST' | 'PUT' | 'PATCH' | 'DELETE'; headers?: Record; body?: string; - expected: { status: number; includes?: string; excludes?: string }; + expected: { status: number | number[]; includes?: string; excludes?: string }; vulnerable?: { status: number; includes?: string; excludes?: string }; } export interface Command { @@ -648,16 +648,34 @@ export function validateVerificationObservation(value: unknown): VerificationObs inputHash: v.inputHash, }; } -function assertion(value: unknown, name: string): HttpAssertion { +const MAX_SAFE_STATUSES = 8; +function assertion(value: unknown, name: string, safeStatusSet = false): HttpAssertion { const v = object(value, name), expected = object(v.expected, `${name}.expected`); exact(v, ['name', 'path', 'method', 'headers', 'body', 'expected', 'vulnerable'], name); - const oracle = (x: Record, n: string) => { + const httpStatus = (s: unknown) => Number.isInteger(s) && (s as number) >= 100 && (s as number) <= 599; + const oracle = (x: Record, n: string, statusSet = false) => { exact(x, ['status', 'includes', 'excludes'], n); - if (!Number.isInteger(x.status) || x.status < 100 || x.status > 599) - throw new CsoError('INVALID_SCHEMA', `${n}.status must be an HTTP status`); + if (!Array.isArray(x.status)) { + if (!httpStatus(x.status)) throw new CsoError('INVALID_SCHEMA', `${n}.status must be an HTTP status`); + } else if (!statusSet) throw new CsoError('INVALID_SCHEMA', `${n}.status must be an HTTP status`); + else if ( + !x.status.length || + x.status.length > MAX_SAFE_STATUSES || + !x.status.every(httpStatus) || + new Set(x.status).size !== x.status.length + ) + throw new CsoError( + 'INVALID_SCHEMA', + `${n}.status must be 1..${MAX_SAFE_STATUSES} distinct HTTP statuses`, + ); + else if (typeof x.excludes !== 'string' || !x.excludes) + throw new CsoError( + 'INVALID_SCHEMA', + `${n} with a set of safe statuses must exclude the vulnerable marker`, + ); return { - status: x.status, + status: Array.isArray(x.status) ? [...x.status] : x.status, ...(x.includes === undefined ? {} : { includes: string(x.includes, `${n}.includes`) }), ...(x.excludes === undefined ? {} : { excludes: string(x.excludes, `${n}.excludes`) }), }; @@ -683,7 +701,7 @@ function assertion(value: unknown, name: string): HttpAssertion { method: oneOf(v.method, ['GET', 'POST', 'PUT', 'PATCH', 'DELETE'], `${name}.method`), ...(Object.keys(headers).length ? { headers } : {}), ...(v.body === undefined ? {} : { body: string(v.body, `${name}.body`, 65536) }), - expected: oracle(expected, `${name}.expected`), + expected: oracle(expected, `${name}.expected`, safeStatusSet && v.vulnerable !== undefined), ...(v.vulnerable === undefined ? {} : { vulnerable: oracle(object(v.vulnerable), `${name}.vulnerable`) }), }; } @@ -692,13 +710,13 @@ export function securityAssertions(security: VerificationRequest['security']): H return Array.isArray(security) ? security : [security]; } function securityAssertion(value: unknown): VerificationRequest['security'] { - if (!Array.isArray(value)) return assertion(value, 'security'); + if (!Array.isArray(value)) return assertion(value, 'security', true); if (!value.length || value.length > MAX_SECURITY_ASSERTIONS) throw new CsoError( 'INVALID_SCHEMA', `security must be one assertion or 1..${MAX_SECURITY_ASSERTIONS} assertions`, ); - const assertions = value.map((x, i) => assertion(x, `security[${i}]`)); + const assertions = value.map((x, i) => assertion(x, `security[${i}]`, true)); if (new Set(assertions.map((x) => x.name)).size !== assertions.length) throw new CsoError('INVALID_SCHEMA', 'security assertion names must be distinct'); return assertions; @@ -805,7 +823,9 @@ export function validateVerificationRequest(input: unknown): VerificationRequest 'Verification needs a legitimate control, distinct before/fixed security oracles, existing tests, immutable test files, and boundary files', ); const mutuallyExclusive = ({ expected: secure, vulnerable }: HttpAssertion) => - secure.status !== vulnerable!.status || + (Array.isArray(secure.status) + ? !secure.status.includes(vulnerable!.status) + : secure.status !== vulnerable!.status) || (secure.includes !== undefined && vulnerable!.excludes !== undefined && secure.includes.includes(vulnerable!.excludes)) || diff --git a/lib/cso/verifier.ts b/lib/cso/verifier.ts index f3364b764..88b61a3e0 100644 --- a/lib/cso/verifier.ts +++ b/lib/cso/verifier.ts @@ -18,7 +18,7 @@ interface Config { } function matches(status: number, body: string, oracle: HttpAssertion['expected']): boolean { return ( - status === oracle.status && + (Array.isArray(oracle.status) ? oracle.status.includes(status) : status === oracle.status) && (oracle.includes === undefined || body.includes(oracle.includes)) && (oracle.excludes === undefined || !body.includes(oracle.excludes)) ); diff --git a/test/cso-contracts.test.ts b/test/cso-contracts.test.ts index fa5584980..3f8f452be 100644 --- a/test/cso-contracts.test.ts +++ b/test/cso-contracts.test.ts @@ -194,6 +194,17 @@ describe('tested repair certificate gate',()=>{ const runDir=tmp(),snapshot=path.join(runDir,'snapshot');fs.mkdirSync(snapshot);const files:Record={'package.json':JSON.stringify({scripts:{test:'jest'}}),'package-lock.json':'{}\n','app.js':'server.listen(3456)\n','app.test.js':'test("ok",()=>{})\n'};for(const [name,body] of Object.entries(files))fs.writeFileSync(path.join(snapshot,name),body);const plan=canonicalTestPlan(snapshot,'node'),raw:any=request(files['app.js'],'server.listen(3456)\n');raw.start={executable:'/usr/local/bin/node',args:['app.js']};raw.existingTests=plan.commands;raw.testFiles=plan.files;raw.changes=[{path:'package-lock.json',beforeSha256:sha256(files['package-lock.json']),after:'{"changed":true}\n',effect:'dependency'}];raw.review.reviewedPatchHash=patchHash(raw);const entries=Object.entries(files).map(([path,body])=>({path,originalHash:sha256(body),executionHash:sha256(body),bytes:Buffer.byteLength(body),mode:0o600})),manifest:any={version:3,createdAt:'2026-01-01T00:00:00Z',expiresAt:'2026-01-08T00:00:00Z',root:'/repo',headCommit:'a'.repeat(40),originalHash:'b'.repeat(64),executionHash:treeHash(snapshot),entries};let executed=false;await expect(verifyRepair({runId:'run',runDir,manifest,rawRequest:raw,runtime:{id:'node',stack:'node',image:'runtime@sha256:'+'a'.repeat(64),platform:'linux/amd64'} as any,verifier:{id:'node',stack:'node',image:'runtime@sha256:'+'a'.repeat(64),platform:'linux/amd64'} as any,policyHash:'c'.repeat(64),archives:[],executor:{observe:async()=>{executed=true;throw new Error('must not run');}},persist:false})).rejects.toMatchObject({code:'PREREQUISITE'});expect(executed).toBe(false); }); test('overlapping security oracles and no-op repairs cannot be certified',()=>{const overlapping:any=request('vulnerable\n','fixed\n');overlapping.review.reviewedPatchHash='pending';overlapping.security.vulnerable={...overlapping.security.expected};expect(()=>validateVerificationRequest(overlapping)).toThrow('mutually exclusive');const noop:any=request('vulnerable\n','vulnerable\n');noop.review.reviewedPatchHash='pending';expect(()=>validateVerificationRequest(noop)).toThrow('material patch');}); + test('a security expectation may list several safe statuses only while it excludes the vulnerable marker',()=>{ + const make=(edit:(raw:any)=>void)=>{const raw:any=request('vulnerable\n','fixed\n');raw.review.reviewedPatchHash='pending';edit(raw);return raw;}; + expect(validateVerificationRequest(make(raw=>{raw.security.expected={status:[403,404],excludes:'tenant-b'};})).security).toMatchObject({expected:{status:[403,404],excludes:'tenant-b'}}); + expect(validateVerificationRequest(make(raw=>{raw.security.expected={status:[200,403],excludes:'tenant-b'};})).security).toMatchObject({expected:{status:[200,403]}}); + expect(()=>validateVerificationRequest(make(raw=>{raw.security.expected={status:[200,403],excludes:'unrelated'};}))).toThrow('mutually exclusive'); + expect(()=>validateVerificationRequest(make(raw=>{raw.security.expected={status:[403,404]};}))).toThrow('must exclude the vulnerable marker'); + for(const status of [[],[403,403],[99,403],[400,401,403,404,405,409,410,422,429]])expect(()=>validateVerificationRequest(make(raw=>{raw.security.expected={status,excludes:'tenant-b'};}))).toThrow('distinct HTTP statuses'); + expect(()=>validateVerificationRequest(make(raw=>{raw.security.vulnerable={status:[200,206],includes:'tenant-b'};}))).toThrow('security.vulnerable.status must be an HTTP status'); + expect(()=>validateVerificationRequest(make(raw=>{raw.legitimate[0].expected={status:[200,403],excludes:'denied'};}))).toThrow('legitimate[0].expected.status must be an HTTP status'); + expect(()=>validateVerificationRequest(make(raw=>{raw.security=[{...raw.security,expected:{status:[403,404],excludes:'tenant-b'}},{...raw.security,name:'second',expected:{status:[403,404]}}];}))).toThrow('security[1].expected with a set of safe statuses'); + }); test('review is a separate content-bound but explicitly self-attested artifact',()=>{const raw:any=request('vulnerable\n','fixed\n');raw.review.reviewedPatchHash=patchHash(raw);const req=validateVerificationRequest(raw),artifact=makeReviewArtifact('run',req,'producer-session');raw.review.artifactId=artifact.id;const bound=validateVerificationRequest(raw);expect(validateReviewArtifact(artifact,'run',bound)).toMatchObject({producer:'producer-session',assurance:'self_attested'});expect(()=>makeReviewArtifact('run',req,req.review.reviewer)).toThrow('distinct');expect(()=>validateReviewArtifact({...artifact,rationale:'changed'},'run',bound)).toThrow('does not bind');}); test('executor observations have a strict helper-owned shape',()=>{expect(()=>validateVerificationObservation({booted:true,legitimate:true,security:'pass',existingTests:true,output:'ok',inputHash:'',tested:true})).toThrow('Unexpected');expect(()=>validateVerificationObservation({booted:'yes',legitimate:true,security:'pass',existingTests:true,output:'ok',inputHash:''})).toThrow('booleans');}); test('secret-bearing fixtures cannot enter a replayable verification bundle',async()=>{const runDir=tmp(),snapshot=path.join(runDir,'snapshot');fs.mkdirSync(snapshot);fs.writeFileSync(path.join(snapshot,'app.js'),'vulnerable\n');const raw:any=request('vulnerable\n','fixed\n');raw.review.reviewedPatchHash=patchHash(raw);raw.fixtures={'secret.txt':CREDENTIAL_CANARY};const manifest:any={entries:[{path:'app.js',transformation:undefined}]};const runtime:any={id:'node',image:'runtime@sha256:'+'a'.repeat(64),platform:'linux/amd64'};expect(verifyRepair({runId:'run',runDir,manifest,rawRequest:raw,runtime,verifier:runtime,policyHash:'c'.repeat(64),archives:[],executor:{observe:async()=>{throw new Error('must not execute');}},persist:false})).rejects.toMatchObject({code:'REDACTION_FAILED'});}); diff --git a/test/cso-verifier.test.ts b/test/cso-verifier.test.ts index 48ab1e1b9..95cd725c0 100644 --- a/test/cso-verifier.test.ts +++ b/test/cso-verifier.test.ts @@ -65,6 +65,19 @@ describe('CSO verifier security judgement',()=>{ try{expect((await judgeSecurity('before',payloads,disproved.port)).security).toBe('pass');}finally{disproved.server.stop(true);} }); + test('a set of safe statuses passes any listed rejection that withholds the marker and nothing else',async()=>{ + const dotfile:HttpAssertion={name:'dotfile',path:'/.env',method:'GET',expected:{status:[403,404],excludes:'DOTFILE'},vulnerable:{status:200,includes:'DOTFILE'}}; + const answer=async(status:number,body:string)=>{ + const server=Bun.serve({port:0,hostname:'127.0.0.1',fetch:()=>new Response(body,{status})}); + try{return [(await judgeSecurity('before',dotfile,Number(server.port))).security,(await judgeSecurity('after',dotfile,Number(server.port))).security];}finally{server.stop(true);} + }; + expect(await answer(404,'not found')).toEqual(['pass','pass']); + expect(await answer(403,'denied')).toEqual(['pass','pass']); + expect(await answer(200,'DOTFILE=1')).toEqual(['intended_failure','inconclusive']); + expect(await answer(403,'denied DOTFILE=1')).toEqual(['inconclusive','inconclusive']); + expect(await answer(400,'bad request')).toEqual(['inconclusive','inconclusive']); + }); + test('a single assertion keeps its original before and after outcomes',async()=>{ const vulnerable=serve(new Set()),fixed=serve(ALL); try{