Skip to content

Commit 26debd4

Browse files
committed
fix: harden package trust boundaries
Reject symlinked manifest/runtime paths and require explicit credential-bearing child processes, including separation of source-read and mutation tokens.\n\nCloses #2075\n\nCo-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 264b8d2 commit 26debd4

7 files changed

Lines changed: 74 additions & 13 deletions

File tree

‎scripts/gh-aw-hosted-e2e.mjs‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,13 @@ function requireArg(args, name) {
6767
return value;
6868
}
6969

70-
export function sanitizedEnvironment(extra = {}) {
70+
export function sanitizedEnvironment(extra = {}, { allowGitHubToken = false } = {}) {
71+
if ('SQUAD_GH_AW_E2E_TOKEN' in extra) {
72+
throw new Error('SQUAD_GH_AW_E2E_TOKEN must never be passed to a child process.');
73+
}
74+
if ('GH_TOKEN' in extra && !allowGitHubToken) {
75+
throw new Error('GH_TOKEN requires an explicitly privileged or source-read child process.');
76+
}
7177
const env = Object.fromEntries(
7278
SAFE_CHILD_ENV.filter((key) => process.env[key] !== undefined)
7379
.map((key) => [key, process.env[key]]),
@@ -79,7 +85,7 @@ export function runChild(command, args, options = {}) {
7985
const result = spawnSync(command, args, {
8086
cwd: options.cwd,
8187
encoding: 'utf8',
82-
env: sanitizedEnvironment(options.env),
88+
env: sanitizedEnvironment(options.env, { allowGitHubToken: options.allowGitHubToken === true }),
8389
stdio: options.capture === false ? 'inherit' : 'pipe',
8490
timeout: options.timeout ?? 120_000,
8591
});
@@ -95,7 +101,11 @@ export function runChild(command, args, options = {}) {
95101
function privileged(command, args, options = {}) {
96102
const token = process.env.GH_TOKEN;
97103
if (!token) throw new Error('GH_TOKEN is required for the trusted hosted journey.');
98-
return runChild(command, args, { ...options, env: { ...options.env, GH_TOKEN: token } });
104+
return runChild(command, args, {
105+
...options,
106+
allowGitHubToken: true,
107+
env: { ...options.env, GH_TOKEN: token },
108+
});
99109
}
100110

101111
function ghJson(args, options = {}) {
@@ -106,7 +116,7 @@ function ghJson(args, options = {}) {
106116
function sourceGhJson(args) {
107117
const token = process.env.SOURCE_READ_TOKEN ?? process.env.GH_TOKEN;
108118
if (!token) throw new Error('A source repository read token is required.');
109-
const output = runChild('gh', args, { env: { GH_TOKEN: token } });
119+
const output = runChild('gh', args, { allowGitHubToken: true, env: { GH_TOKEN: token } });
110120
return output ? JSON.parse(output) : null;
111121
}
112122

@@ -339,10 +349,14 @@ function hosted(args, repositoryRoot) {
339349

340350
const sourceReadToken = process.env.SOURCE_READ_TOKEN;
341351
if (!sourceReadToken) throw new Error('SOURCE_READ_TOKEN is required for immutable package retrieval.');
352+
if (sourceReadToken === process.env.GH_TOKEN) {
353+
throw new Error('SOURCE_READ_TOKEN must not reuse the mutation-capable GH_TOKEN.');
354+
}
342355
runChild('gh', ['aw', 'add', `${PACKAGE_NAME}@${sourceSha}`], {
343356
cwd: checkout,
344357
capture: false,
345358
timeout: 600_000,
359+
allowGitHubToken: true,
346360
env: { GH_TOKEN: sourceReadToken },
347361
});
348362
runChild('node', ['.github/workflows/shared/squad-install-verifier.mjs', '--materialize-runtime'], {

‎test/gh-aw-hosted-e2e.test.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ describe('Squad gh-aw hosted E2E controller', () => {
3737
expect(env.OTHER_CREDENTIAL).toBeUndefined();
3838
expect(env.ARBITRARY_UNTRUSTED_VALUE).toBeUndefined();
3939
expect(env.SAFE_VALUE).toBe('ok');
40+
expect(() => sanitizedEnvironment({ GH_TOKEN: 'secret' })).toThrow(/explicitly privileged/);
41+
expect(() => sanitizedEnvironment({ SQUAD_GH_AW_E2E_TOKEN: 'secret' })).toThrow(/never be passed/);
4042
delete process.env.GH_TOKEN;
4143
delete process.env.SQUAD_GH_AW_E2E_TOKEN;
4244
delete process.env.OTHER_CREDENTIAL;
@@ -91,8 +93,12 @@ describe('Squad gh-aw hosted E2E controller', () => {
9193
});
9294

9395
it('never executes candidate package or installed verifier with inherited credentials', () => {
94-
expect(SCRIPT).toContain('env: sanitizedEnvironment(options.env)');
96+
expect(SCRIPT).toContain(
97+
'env: sanitizedEnvironment(options.env, { allowGitHubToken: options.allowGitHubToken === true })',
98+
);
9599
expect(SCRIPT).toContain("runChild('gh', ['aw', 'add'");
100+
expect(SCRIPT).toContain('SOURCE_READ_TOKEN must not reuse the mutation-capable GH_TOKEN');
101+
expect(SCRIPT).toContain('allowGitHubToken: true');
96102
expect(SCRIPT).toContain('env: { GH_TOKEN: sourceReadToken }');
97103
expect(SCRIPT).toContain("runChild('node', ['.github/workflows/shared/squad-install-verifier.mjs'");
98104
expect(SCRIPT).not.toMatch(/privileged\('node'/);

‎test/gh-aw-quality.test.ts‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
*/
1010

1111
import { afterAll, describe, it, expect } from 'vitest';
12-
import { chmodSync, cpSync, readFileSync, existsSync, mkdirSync, mkdtempSync, writeFileSync, rmSync, unlinkSync } from 'node:fs';
12+
import { chmodSync, cpSync, readFileSync, existsSync, mkdirSync, mkdtempSync, writeFileSync, rmSync, symlinkSync, unlinkSync } from 'node:fs';
1313
import { join, dirname } from 'node:path';
1414
import { tmpdir } from 'node:os';
1515
import { execFileSync, execSync, spawnSync } from 'node:child_process';
@@ -3670,6 +3670,24 @@ describe('gh-aw: canonical package integrity contract', () => {
36703670
}
36713671
});
36723672

3673+
it('rejects symlinked manifest leaves and runtime parents', () => {
3674+
const manifestRoot = makeConsumer();
3675+
const manifestPath = join(manifestRoot, CONTRACT_DESTINATION);
3676+
const manifestTarget = join(manifestRoot, 'redirected-manifest.json');
3677+
writeFileSync(manifestTarget, readFileSync(manifestPath));
3678+
unlinkSync(manifestPath);
3679+
symlinkSync(manifestTarget, manifestPath);
3680+
expect(verifyInstall(manifestRoot).failures.join('\n')).toContain('symbolic link');
3681+
3682+
const runtimeRoot = makeConsumer(revisionA, false);
3683+
const shared = join(runtimeRoot, '.github/workflows/shared');
3684+
const redirected = join(runtimeRoot, '.github/workflows/shared-redirected');
3685+
cpSync(shared, redirected, { recursive: true });
3686+
rmSync(shared, { recursive: true });
3687+
symlinkSync(redirected, shared, 'dir');
3688+
expect(() => materializeRuntime(runtimeRoot)).toThrow(/symbolic link/);
3689+
});
3690+
36733691
it('materializes package-only runtime bytes and rejects stale sources', () => {
36743692
const root = makeConsumer(revisionA, false);
36753693
const contract = JSON.parse(readFileSync(join(root, CONTRACT_DESTINATION), 'utf8'));

‎workflows/package/squad-bootstrap.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ pre-agent-steps:
151151
node --check "$path" >/dev/null
152152
}
153153
# BEGIN GENERATED RESOURCE DIGESTS
154-
check_hash "$install_verifier" "f2af6cebe2aecf1aa152f6871aa428fa5382b1027c92c1eaf4a49f767a4ba4a4"
154+
check_hash "$install_verifier" "0da2b31a80b47e428cb215a00d4655b54b7b66e4a6a2522f0f4da8f119d0346e"
155155
check_hash "$cast_validator" "62fbf47b51639fd1878c143e5176ee3099e390065997411511e9d483d467bbce"
156156
check_hash "$bootstrap_validator" "d449b9204f7fad133ff7133c1a30c9381c87e3c0c9d481352819ca93ea1a1dad"
157157
# END GENERATED RESOURCE DIGESTS

‎workflows/shared/squad-install-verifier.mjs‎

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,11 @@
33
import {
44
cpSync,
55
existsSync,
6+
lstatSync,
67
mkdirSync,
78
mkdtempSync,
89
readFileSync,
10+
realpathSync,
911
readdirSync,
1012
rmSync,
1113
writeFileSync,
@@ -207,24 +209,45 @@ export function validateContract(contract) {
207209

208210
function safePath(root, relativePath) {
209211
validatePathSyntax(relativePath, undefined, relativePath);
210-
const absoluteRoot = resolve(root);
212+
const absoluteRoot = realpathSync(resolve(root));
211213
const candidate = resolve(absoluteRoot, relativePath);
212214
if (candidate !== absoluteRoot && !candidate.startsWith(`${absoluteRoot}${sep}`)) {
213215
throw new Error(`Path escapes repository root: ${relativePath}`);
214216
}
217+
let current = absoluteRoot;
218+
const segments = relativePath.split('/');
219+
for (let index = 0; index < segments.length; index += 1) {
220+
current = join(current, segments[index]);
221+
const stat = lstatSync(current, { throwIfNoEntry: false });
222+
if (!stat) break;
223+
if (stat.isSymbolicLink()) throw new Error(`Path contains a symbolic link: ${relativePath}`);
224+
if (index < segments.length - 1 && !stat.isDirectory()) {
225+
throw new Error(`Path parent is not a directory: ${relativePath}`);
226+
}
227+
}
215228
return candidate;
216229
}
217230

218231
function readRequired(root, relativePath) {
219232
const path = safePath(root, relativePath);
220-
if (!existsSync(path)) throw new Error(`Required file is missing: ${relativePath}`);
233+
const stat = lstatSync(path, { throwIfNoEntry: false });
234+
if (!stat) throw new Error(`Required file is missing: ${relativePath}`);
235+
if (!stat.isFile()) throw new Error(`Required path is not a regular file: ${relativePath}`);
221236
return readFileSync(path);
222237
}
223238

224239
function writePath(root, relativePath, content) {
225-
const path = safePath(root, relativePath);
240+
let path = safePath(root, relativePath);
226241
mkdirSync(dirname(path), { recursive: true });
242+
path = safePath(root, relativePath);
243+
const existing = lstatSync(path, { throwIfNoEntry: false });
244+
if (existing && !existing.isFile()) {
245+
throw new Error(`Write destination is not a regular file: ${relativePath}`);
246+
}
227247
writeFileSync(path, content);
248+
if (!lstatSync(path).isFile()) {
249+
throw new Error(`Written destination is not a regular file: ${relativePath}`);
250+
}
228251
}
229252

230253
function fileDigest(root, path) {

‎workflows/squad-bootstrap.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ pre-agent-steps:
158158
node --check "$path" >/dev/null
159159
}
160160
# BEGIN GENERATED RESOURCE DIGESTS
161-
check_hash "$install_verifier" "f2af6cebe2aecf1aa152f6871aa428fa5382b1027c92c1eaf4a49f767a4ba4a4"
161+
check_hash "$install_verifier" "0da2b31a80b47e428cb215a00d4655b54b7b66e4a6a2522f0f4da8f119d0346e"
162162
check_hash "$cast_validator" "62fbf47b51639fd1878c143e5176ee3099e390065997411511e9d483d467bbce"
163163
check_hash "$bootstrap_validator" "d449b9204f7fad133ff7133c1a30c9381c87e3c0c9d481352819ca93ea1a1dad"
164164
# END GENERATED RESOURCE DIGESTS

‎workflows/squad-workflows.manifest.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@
5656
"source": "workflows/package/squad-bootstrap.md",
5757
"destination": ".github/workflows/squad-bootstrap.md",
5858
"lock": ".github/workflows/squad-bootstrap.lock.yml",
59-
"source_sha256": "4a2cd73cb3395c512eec6e8620fc20387f47607604e87dc6bec66bf96f81b6c2"
59+
"source_sha256": "fb1bc0f0c1f63e09f94892f99bfe857499803a7ec9465108cb128b99eb01a852"
6060
}
6161
],
6262
"shared_runtime": [
@@ -66,7 +66,7 @@
6666
"package_destination": ".github/workflows/shared/squad-install-verifier.mjs",
6767
"destination": ".github/workflows/shared/squad-install-verifier.mjs",
6868
"ownership": "manifest",
69-
"sha256": "f2af6cebe2aecf1aa152f6871aa428fa5382b1027c92c1eaf4a49f767a4ba4a4"
69+
"sha256": "0da2b31a80b47e428cb215a00d4655b54b7b66e4a6a2522f0f4da8f119d0346e"
7070
},
7171
{
7272
"path": "shared/squad-cast-validator.mjs",

0 commit comments

Comments
 (0)