Skip to content

Commit c39f282

Browse files
authored
fix: additional filesystem checks (#1799)
Release-As: 6.1.2
1 parent 8188bee commit c39f282

8 files changed

Lines changed: 1269 additions & 1978 deletions

File tree

package-lock.json

Lines changed: 936 additions & 1954 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
"@biomejs/biome": "2.4.13",
2222
"@smithy/property-provider": "^4.3.4",
2323
"@types/node": "^25.9.1",
24-
"@vitest/coverage-v8": "^3.2.4",
24+
"@vitest/coverage-v8": "^4.1.6",
2525
"aws-sdk-client-mock": "^4.1.0",
2626
"esbuild": "^0.28.0",
2727
"generate-license-file": "^4.2.1",
@@ -30,7 +30,7 @@
3030
"memfs": "^4.57.2",
3131
"standard-version": "^9.5.0",
3232
"typescript": "^6.0.3",
33-
"vitest": "^3.2.4"
33+
"vitest": "^4.1.6"
3434
},
3535
"dependencies": {
3636
"@actions/core": "^2.0.2",

src/assumeRole.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
import assert from 'node:assert';
2-
import fs from 'node:fs';
32
import path from 'node:path';
43
import * as core from '@actions/core';
54
import type { AssumeRoleCommandInput, STSClient, Tag } from '@aws-sdk/client-sts';
65
import { AssumeRoleCommand, AssumeRoleWithWebIdentityCommand } from '@aws-sdk/client-sts';
76
import type { CredentialsClient } from './CredentialsClient';
8-
import { errorMessage, isDefined, sanitizeGitHubVariables } from './helpers';
7+
import { errorMessage, isDefined, readFileUtf8, sanitizeGitHubVariables } from './helpers';
98

109
async function assumeRoleWithOIDC(params: AssumeRoleCommandInput, client: STSClient, webIdentityToken: string) {
1110
delete params.Tags;
@@ -36,12 +35,12 @@ async function assumeRoleWithWebIdentityTokenFile(
3635
const webIdentityTokenFilePath = path.isAbsolute(webIdentityTokenFile)
3736
? webIdentityTokenFile
3837
: path.join(workspace, webIdentityTokenFile);
39-
if (!fs.existsSync(webIdentityTokenFilePath)) {
38+
const webIdentityToken = readFileUtf8(webIdentityTokenFilePath);
39+
if (webIdentityToken === null) {
4040
throw new Error(`Web identity token file does not exist: ${webIdentityTokenFilePath}`);
4141
}
4242
core.info('Assuming role with web identity token file');
4343
try {
44-
const webIdentityToken = fs.readFileSync(webIdentityTokenFilePath, 'utf8');
4544
delete params.Tags;
4645
const creds = await client.send(
4746
new AssumeRoleWithWebIdentityCommand({

src/helpers.ts

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import * as fs from 'node:fs';
2+
import * as path from 'node:path';
13
import * as core from '@actions/core';
24
import type { Credentials, STSClient } from '@aws-sdk/client-sts';
35
import { GetCallerIdentityCommand } from '@aws-sdk/client-sts';
@@ -268,3 +270,88 @@ export function getBooleanInput(name: string, options?: core.InputOptions & { de
268270
`Support boolean input list: \`true | True | TRUE | false | False | FALSE\``,
269271
);
270272
}
273+
274+
// O_NOFOLLOW is undefined on Windows. This sets it to 0 if it's not defined.
275+
const O_NOFOLLOW: number = (fs.constants as { O_NOFOLLOW?: number }).O_NOFOLLOW ?? 0;
276+
277+
export function isSymlink(filePath: string): boolean {
278+
try {
279+
return fs.lstatSync(filePath).isSymbolicLink();
280+
} catch (err) {
281+
if ((err as NodeJS.ErrnoException).code === 'ENOENT') return false;
282+
throw err;
283+
}
284+
}
285+
286+
// Refuses if filePath or its parent directory is a symbolic link.
287+
function refuseSymlinkOnPath(filePath: string): void {
288+
const parent = path.dirname(filePath);
289+
if (parent !== filePath && isSymlink(parent)) {
290+
throw new Error(`Refusing ${filePath} (parent directory is a symbolic link)`);
291+
}
292+
if (isSymlink(filePath)) {
293+
throw new Error(`Refusing ${filePath} (path is a symbolic link)`);
294+
}
295+
}
296+
297+
function assertRegularFile(fd: number, filePath: string): void {
298+
const stats = fs.fstatSync(fd);
299+
if (!stats.isFile()) {
300+
throw new Error(`${filePath} (path is not a regular file)`);
301+
}
302+
}
303+
304+
// ENOENT: file does not exist
305+
// ELOOP: too many symbolic links (from NOFOLLOW)
306+
307+
export function readFileUtf8(filePath: string): string | null {
308+
refuseSymlinkOnPath(filePath);
309+
let fd: number;
310+
try {
311+
fd = fs.openSync(filePath, fs.constants.O_RDONLY | O_NOFOLLOW);
312+
} catch (err) {
313+
const code = (err as NodeJS.ErrnoException).code;
314+
if (code === 'ENOENT') return null;
315+
if (code === 'ELOOP') {
316+
throw new Error(`Refusing ${filePath} (path is a symbolic link)`);
317+
}
318+
throw err;
319+
}
320+
try {
321+
assertRegularFile(fd, filePath);
322+
return fs.readFileSync(fd, 'utf-8');
323+
} finally {
324+
fs.closeSync(fd);
325+
}
326+
}
327+
328+
export function writeFileUtf8(filePath: string, content: string, mode = 0o600): void {
329+
refuseSymlinkOnPath(filePath);
330+
let fd: number;
331+
try {
332+
fd = fs.openSync(filePath, fs.constants.O_WRONLY | fs.constants.O_CREAT | fs.constants.O_TRUNC | O_NOFOLLOW, mode);
333+
} catch (err) {
334+
if ((err as NodeJS.ErrnoException).code === 'ELOOP') {
335+
throw new Error(`Refusing ${filePath} (path is a symbolic link)`);
336+
}
337+
throw err;
338+
}
339+
try {
340+
assertRegularFile(fd, filePath);
341+
// openSync only applies mode on creation.
342+
// If the file already exists, we need to ensure the mode is correct.
343+
if (process.platform !== 'win32') {
344+
fs.fchmodSync(fd, mode);
345+
}
346+
fs.writeFileSync(fd, content);
347+
} finally {
348+
fs.closeSync(fd);
349+
}
350+
}
351+
352+
export function mkdir(dir: string, mode = 0o700): void {
353+
fs.mkdirSync(dir, { recursive: true, mode });
354+
if (isSymlink(dir)) {
355+
throw new Error(`Refusing ${dir} (path is a symbolic link)`);
356+
}
357+
}

src/profileManager.ts

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
import * as fs from 'node:fs';
21
import * as os from 'node:os';
32
import * as path from 'node:path';
43
import * as core from '@actions/core';
54
import type { Credentials } from '@aws-sdk/client-sts';
5+
import { mkdir, readFileUtf8, writeFileUtf8 } from './helpers';
66

77
/**
88
* Parse an INI-format string into a nested object.
@@ -87,10 +87,8 @@ export function getProfileFilePaths(): ProfileFilePaths {
8787
*/
8888
export function ensureAwsDirectoryExists(filePath: string): void {
8989
const dir = path.dirname(filePath);
90-
if (!fs.existsSync(dir)) {
91-
core.debug(`Creating directory: ${dir}`);
92-
fs.mkdirSync(dir, { recursive: true, mode: 0o700 });
93-
}
90+
core.debug(`Ensuring directory exists: ${dir}`);
91+
mkdir(dir, 0o700);
9492
}
9593

9694
/**
@@ -127,14 +125,8 @@ export function mergeProfileSection(
127125
data: Record<string, string>,
128126
overwriteAwsProfile: boolean,
129127
): void {
130-
let existingContent: Record<string, Record<string, string>> = {};
131-
132-
// Read existing file if it exists
133-
if (fs.existsSync(filePath)) {
134-
core.debug(`Reading existing file: ${filePath}`);
135-
const fileContent = fs.readFileSync(filePath, 'utf-8');
136-
existingContent = parseIni(fileContent);
137-
}
128+
const fileContent = readFileUtf8(filePath);
129+
const existingContent: Record<string, Record<string, string>> = fileContent === null ? {} : parseIni(fileContent);
138130

139131
if (existingContent[sectionName] && !overwriteAwsProfile) {
140132
throw new Error(
@@ -147,7 +139,7 @@ export function mergeProfileSection(
147139
const content = stringifyIni(existingContent);
148140

149141
core.debug(`Writing profile to ${filePath}`);
150-
fs.writeFileSync(filePath, content, { mode: 0o600 });
142+
writeFileUtf8(filePath, content, 0o600);
151143
}
152144

153145
/**

test/assumeRole.test.ts

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
import * as core from '@actions/core';
2+
import {
3+
AssumeRoleWithWebIdentityCommand,
4+
GetCallerIdentityCommand,
5+
STSClient,
6+
} from '@aws-sdk/client-sts';
7+
import { mockClient } from 'aws-sdk-client-mock';
8+
import { fs, vol } from 'memfs';
9+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
10+
import * as helpers from '../src/helpers';
11+
import { run } from '../src/index';
12+
import mocks from './mockinputs.test';
13+
14+
vi.mock('node:fs');
15+
vi.mock('@actions/core');
16+
17+
const mockedSTSClient = mockClient(STSClient);
18+
19+
describe('assumeRoleWithWebIdentityTokenFile', {}, () => {
20+
beforeEach(() => {
21+
vi.restoreAllMocks();
22+
vi.clearAllMocks();
23+
mockedSTSClient.reset();
24+
vol.reset();
25+
helpers.withsleep(() => Promise.resolve());
26+
vi.mocked(core.getInput).mockImplementation(mocks.getInput(mocks.WEBIDENTITY_TOKEN_FILE_INPUTS));
27+
vi.mocked(core.getMultilineInput).mockReturnValue([]);
28+
mockedSTSClient.on(GetCallerIdentityCommand).resolves({ ...mocks.outputs.GET_CALLER_IDENTITY });
29+
process.env = { ...mocks.envs };
30+
fs.mkdirSync('/home/github', { recursive: true });
31+
});
32+
33+
afterEach(() => {
34+
helpers.reset();
35+
});
36+
37+
it('refuses when the token file is a symlink and never calls STS', async () => {
38+
fs.mkdirSync('/etc', { recursive: true });
39+
fs.writeFileSync('/etc/passwd', 'root:x:0:0::/root:/bin/sh');
40+
fs.symlinkSync('/etc/passwd', '/home/github/file.txt');
41+
42+
await run();
43+
44+
expect(core.setFailed).toHaveBeenCalledWith(expect.stringMatching(/Refusing .* \(.* symbolic link\)/));
45+
expect(mockedSTSClient.commandCalls(AssumeRoleWithWebIdentityCommand)).toHaveLength(0);
46+
expect(fs.readFileSync('/etc/passwd', 'utf-8')).toBe('root:x:0:0::/root:/bin/sh');
47+
});
48+
49+
it('preserves the existing missing-file error when the token file does not exist', async () => {
50+
await run();
51+
52+
expect(core.setFailed).toHaveBeenCalledWith(expect.stringContaining('Web identity token file does not exist'));
53+
expect(mockedSTSClient.commandCalls(AssumeRoleWithWebIdentityCommand)).toHaveLength(0);
54+
});
55+
56+
it('passes token contents to STS when the file is regular', async () => {
57+
fs.writeFileSync('/home/github/file.txt', 'real-token');
58+
mockedSTSClient.on(AssumeRoleWithWebIdentityCommand).resolves(mocks.outputs.STS_CREDENTIALS);
59+
60+
await run();
61+
62+
expect(core.setFailed).not.toHaveBeenCalled();
63+
const calls = mockedSTSClient.commandCalls(AssumeRoleWithWebIdentityCommand);
64+
expect(calls).toHaveLength(1);
65+
expect(calls[0]?.args[0].input.WebIdentityToken).toBe('real-token');
66+
});
67+
});

test/helpers.test.ts

Lines changed: 99 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,16 @@
11
import * as core from '@actions/core';
2+
import { fs, vol } from 'memfs';
23
import { beforeEach, describe, expect, it, vi } from 'vitest';
34
import * as helpers from '../src/helpers';
45

6+
vi.mock('node:fs');
7+
vi.mock('@actions/core');
8+
59
describe('Configure AWS Credentials helpers', {}, () => {
610
beforeEach(() => {
711
vi.restoreAllMocks();
8-
vi.spyOn(core, 'debug').mockImplementation(() => {});
12+
vi.clearAllMocks();
13+
vol.reset();
914
});
1015
it('removes brackets from GitHub Actor', {}, () => {
1116
const actor = 'actor[bot]';
@@ -97,4 +102,97 @@ describe('Configure AWS Credentials helpers', {}, () => {
97102
helpers.exportCredentials({ AccessKeyId: 'test', SecretAccessKey: 'test' }, false, true);
98103
expect(core.exportVariable).toHaveBeenCalledWith('AWS_SESSION_TOKEN', '');
99104
});
105+
106+
describe('filesystem helpers', {}, () => {
107+
describe('isSymlink', {}, () => {
108+
it('returns true for a symlink', {}, () => {
109+
fs.mkdirSync('/dir', { recursive: true });
110+
fs.writeFileSync('/dir/target', 'data');
111+
fs.symlinkSync('/dir/target', '/dir/link');
112+
expect(helpers.isSymlink('/dir/link')).toBe(true);
113+
});
114+
115+
it('returns false for a regular file', {}, () => {
116+
fs.mkdirSync('/dir', { recursive: true });
117+
fs.writeFileSync('/dir/file', 'data');
118+
expect(helpers.isSymlink('/dir/file')).toBe(false);
119+
});
120+
121+
it('returns false for a missing path', {}, () => {
122+
expect(helpers.isSymlink('/nonexistent')).toBe(false);
123+
});
124+
});
125+
126+
describe('readFileUtf8', {}, () => {
127+
it('returns content for a regular file', {}, () => {
128+
fs.mkdirSync('/dir', { recursive: true });
129+
fs.writeFileSync('/dir/file', 'hello');
130+
expect(helpers.readFileUtf8('/dir/file')).toBe('hello');
131+
});
132+
133+
it('returns null when the file does not exist', {}, () => {
134+
fs.mkdirSync('/dir', { recursive: true });
135+
expect(helpers.readFileUtf8('/dir/missing')).toBe(null);
136+
});
137+
138+
it('refuses to read through a symlink at the target', {}, () => {
139+
fs.mkdirSync('/dir', { recursive: true });
140+
fs.writeFileSync('/dir/secret', 'sensitive');
141+
fs.symlinkSync('/dir/secret', '/dir/link');
142+
expect(() => helpers.readFileUtf8('/dir/link')).toThrow(/Refusing .* \(.* symbolic link\)/);
143+
});
144+
145+
it('refuses to read when the parent directory is a symlink', {}, () => {
146+
fs.mkdirSync('/real/.aws', { recursive: true });
147+
fs.writeFileSync('/real/.aws/credentials', 'data');
148+
fs.mkdirSync('/home', { recursive: true });
149+
fs.symlinkSync('/real/.aws', '/home/.aws');
150+
expect(() => helpers.readFileUtf8('/home/.aws/credentials')).toThrow(/Refusing .* \(.* symbolic link\)/);
151+
});
152+
153+
it('refuses to read when the path is a directory', {}, () => {
154+
fs.mkdirSync('/dir/subdir', { recursive: true });
155+
expect(() => helpers.readFileUtf8('/dir/subdir')).toThrow(/not a regular file/);
156+
});
157+
});
158+
159+
describe('writeFileUtf8', {}, () => {
160+
it('writes content with the specified mode', {}, () => {
161+
fs.mkdirSync('/dir', { recursive: true });
162+
helpers.writeFileUtf8('/dir/file', 'payload', 0o600);
163+
expect(fs.readFileSync('/dir/file', 'utf-8')).toBe('payload');
164+
expect(fs.statSync('/dir/file').mode & 0o777).toBe(0o600);
165+
});
166+
167+
it('refuses to follow a symlink at the target and leaves the target file untouched', {}, () => {
168+
fs.mkdirSync('/dir', { recursive: true });
169+
fs.writeFileSync('/dir/target', 'original');
170+
fs.symlinkSync('/dir/target', '/dir/link');
171+
expect(() => helpers.writeFileUtf8('/dir/link', 'attacker', 0o600)).toThrow(/Refusing .* \(.* symbolic link\)/);
172+
expect(fs.readFileSync('/dir/target', 'utf-8')).toBe('original');
173+
});
174+
175+
it.skipIf(process.platform === 'win32')('tightens mode on existing files', () => {
176+
fs.mkdirSync('/dir', { recursive: true });
177+
fs.writeFileSync('/dir/file', 'old', { mode: 0o644 });
178+
helpers.writeFileUtf8('/dir/file', 'new', 0o600);
179+
expect(fs.statSync('/dir/file').mode & 0o777).toBe(0o600);
180+
});
181+
});
182+
183+
describe('mkdir', {}, () => {
184+
it('is idempotent on a regular directory', {}, () => {
185+
helpers.mkdir('/some/nested/dir', 0o700);
186+
helpers.mkdir('/some/nested/dir', 0o700);
187+
expect(fs.statSync('/some/nested/dir').isDirectory()).toBe(true);
188+
});
189+
190+
it('refuses when the target directory is a symlink', {}, () => {
191+
fs.mkdirSync('/real', { recursive: true });
192+
fs.mkdirSync('/home', { recursive: true });
193+
fs.symlinkSync('/real', '/home/.aws');
194+
expect(() => helpers.mkdir('/home/.aws', 0o700)).toThrow(/Refusing .* \(.* symbolic link\)/);
195+
});
196+
});
197+
});
100198
});

0 commit comments

Comments
 (0)