Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -123,12 +123,13 @@ Remote-helper overrides (`--upload-pack`, `--receive-pack`, `--exec`, and abbrev
Remote-helper URL forms (`ext::…` and other `scheme::` tokens) are rejected for the same reason.
Git child processes are also limited to the `https`, `http`, `ssh`, `file`, and `git` transports (`GIT_ALLOW_PROTOCOL`) unless you set [`allow_unsafe_git_protocols`](#allow-unsafe-git-protocols) to `true` (only for trusted custom remotes/helpers).
Message-from-file flags (`-F`, `--file`, abbreviations such as `--fi`, and short-option clusters that include `F` such as `-aF`) are rejected: they can embed arbitrary runner filesystem contents into a tag or commit message and, with a push, into the repository history.
Pathspec-from-file flags (`--pathspec-from-file`, `--pathspec-file-nul`, and abbreviations such as `--pathspec-fr` / `--pathspec-fi`) are rejected on `add`, `remove`, and `commit`: they can read an arbitrary runner file and leak its contents into the action log.
Unmatched `'` / `"` quotes are also rejected: `string-argv` can otherwise split on an odd quote and turn part of a value into extra flags (for example a branch name like `fix'--force` becoming `fix` plus `--force`).
Do not interpolate untrusted data (for example values from `github.event.*`, `github.head_ref`, or repository content that contributors can edit) into `fetch`, `pull`, `push`, `tag`, `tag_push`, or `commit` without sanitizing them first. When the branch name is dynamic, prefer the default `push: true` with [`new_branch`](#creating-a-new-branch) instead of embedding the ref in a custom `push` string.

### Allow unsafe git protocols

Set `allow_unsafe_git_protocols: true` only if you need a custom remote helper or a transport outside the default allowlist (`https`, `http`, `ssh`, `file`, `git`). This disables both the `GIT_ALLOW_PROTOCOL` restriction and the rejection of `scheme::` tokens in git argument inputs. It does **not** re-enable blocked options such as `--upload-pack` or `-F`/`--file`. Treat this like a break-glass setting: only enable it with fully trusted, non-interpolated argument strings.
Set `allow_unsafe_git_protocols: true` only if you need a custom remote helper or a transport outside the default allowlist (`https`, `http`, `ssh`, `file`, `git`). This disables both the `GIT_ALLOW_PROTOCOL` restriction and the rejection of `scheme::` tokens in git argument inputs. It does **not** re-enable blocked options such as `--upload-pack`, `-F`/`--file`, or `--pathspec-from-file`/`--pathspec-file-nul`. Treat this like a break-glass setting: only enable it with fully trusted, non-interpolated argument strings.

### Adding files

Expand Down
6 changes: 3 additions & 3 deletions action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ description: Automatically commit changes made in your workflow run directly to

inputs:
add:
description: Arguments for the git add command
description: Arguments for the git add command. --pathspec-from-file/--pathspec-file-nul and abbreviations (e.g. --pathspec-fr, --pathspec-fi) are not allowed.
required: false
default: '.'
author_name:
Expand All @@ -13,7 +13,7 @@ inputs:
description: The email of the user that will be displayed as the author of the commit
required: false
commit:
description: Additional arguments for the git commit command. -F/--file, abbreviations (e.g. --fi), and short-option clusters containing F (e.g. -aF) are not allowed.
description: Additional arguments for the git commit command. -F/--file, abbreviations (e.g. --fi), and short-option clusters containing F (e.g. -aF) are not allowed. --pathspec-from-file/--pathspec-file-nul and abbreviations (e.g. --pathspec-fr, --pathspec-fi) are not allowed.
required: false
committer_name:
description: The name of the custom committer you want to use
Expand Down Expand Up @@ -59,7 +59,7 @@ inputs:
required: false
default: '1'
remove:
description: Arguments for the git rm command
description: Arguments for the git rm command. --pathspec-from-file/--pathspec-file-nul and abbreviations (e.g. --pathspec-fr, --pathspec-fi) are not allowed.
required: false
tag:
description: Arguments for the git tag command (the tag name always needs to be the first word not preceded by a hyphen). -F/--file, abbreviations (e.g. --fi), and short-option clusters containing F (e.g. -aF) are not allowed.
Expand Down
2 changes: 1 addition & 1 deletion lib/index.js

Large diffs are not rendered by default.

27 changes: 26 additions & 1 deletion src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,20 @@ const DANGEROUS_MESSAGE_FILE_OPTIONS: ReadonlyArray<{
minPrefix: string;
}> = [{canonical: 'file', minPrefix: 'fi'}];

/**
* Long options that make Git read pathspecs from a filesystem path.
* Git accepts unique abbreviations (`--pathspec-fr` → `--pathspec-from-file`);
* `minPrefix` is the shortest unambiguous abbreviation currently accepted.
* `--pathspec-file-nul` is blocked with them (it changes how that file is parsed).
*/
const DANGEROUS_PATHSPEC_FILE_OPTIONS: ReadonlyArray<{
canonical: string;
minPrefix: string;
}> = [
{canonical: 'pathspec-from-file', minPrefix: 'pathspec-fr'},
{canonical: 'pathspec-file-nul', minPrefix: 'pathspec-fi'},
];

/**
* Long options whose next argv token is a value, not another option.
* Used so literals like `-m '-F'` are not treated as a message-file flag.
Expand All @@ -242,6 +256,7 @@ const LONG_OPTIONS_WITH_SEPARATE_ARG: ReadonlyArray<{
{canonical: 'local-user', minPrefix: 'local-'},
{canonical: 'cleanup', minPrefix: 'cleanup'},
{canonical: 'file', minPrefix: 'fi'},
{canonical: 'pathspec-from-file', minPrefix: 'pathspec-fr'},
{canonical: 'upload-pack', minPrefix: 'upl'},
{canonical: 'receive-pack', minPrefix: 'rece'},
{canonical: 'exec', minPrefix: 'e'},
Expand Down Expand Up @@ -311,6 +326,10 @@ function isDangerousMessageFileOption(arg: string): boolean {
);
}

function isDangerousPathspecFileOption(arg: string): boolean {
return matchesLongOptionPrefix(arg, DANGEROUS_PATHSPEC_FILE_OPTIONS);
}

/**
* Whether this token causes Git to treat the next argv element as a value
* (so that value must not be classified as an option).
Expand Down Expand Up @@ -368,7 +387,7 @@ function isRemoteHelperUrl(arg: string): boolean {
export type MatchGitArgsOptions = {
/**
* When true, allow `scheme::` remote-helper URL tokens.
* Does not disable `--upload-pack` / `-F` denylists.
* Does not disable `--upload-pack` / `-F` / `--pathspec-from-file` denylists.
*/
allowUnsafeGitProtocols?: boolean;
};
Expand Down Expand Up @@ -400,6 +419,7 @@ export type MatchGitArgsOptions = {
* @throws If the args include unmatched quotes
* @throws If the args include a blocked remote-helper override (`--upload-pack`, `--receive-pack`, `--exec`, or abbreviations) on any token, including values after `-u` / `-m`
* @throws If the args include a blocked message-from-file flag (`-F`, `--file`, abbreviations, or short-option clusters containing `F`)
* @throws If the args include a blocked pathspec-from-file flag (`--pathspec-from-file`, `--pathspec-file-nul`, or abbreviations)
* @throws If the args include a `scheme::` remote-helper URL (unless `allowUnsafeGitProtocols`)
*/
export function matchGitArgs(
Expand Down Expand Up @@ -435,6 +455,11 @@ export function matchGitArgs(
`Git argument '${arg}' is not allowed: reading a tag/commit message from a file (-F/--file) can exfiltrate runner filesystem contents into git history.`,
);
}
if (isDangerousPathspecFileOption(arg)) {
throw new Error(
`Git argument '${arg}' is not allowed: reading pathspecs from a file (--pathspec-from-file/--pathspec-file-nul) can leak runner filesystem contents into logs.`,
);
}
if (!allowUnsafe && isRemoteHelperUrl(arg)) {
throw new Error(
`Git argument '${arg}' is not allowed: remote-helper URLs (scheme::…) can execute arbitrary commands on the runner. Set allow_unsafe_git_protocols to true only if you fully trust this input.`,
Expand Down
120 changes: 120 additions & 0 deletions test/integration/action.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -409,4 +409,124 @@ describe('action integration', () => {
expect(combined).not.toMatch(/function isObject/);
expect(combined.length).toBeLessThan(50_000);
});

describe('rejects --pathspec-from-file / --pathspec-file-nul', () => {
const MARKER_A = 'AAC_POC_SECRET_7f9c2e';
const MARKER_B = 'AAC_SECOND_LINE_91a4d8';

function writeDummyOutsideClone(f: Fixture): string {
const dummyPath = path.join(
path.dirname(f.local),
`pathspec-dummy-${process.pid}.txt`,
);
fs.writeFileSync(dummyPath, `${MARKER_A}\n${MARKER_B}\n`);
return dummyPath;
}

function pathspecFromFileArgs(dummyPath: string): string {
return `--pathspec-from-file=${dummyPath} --pathspec-file-nul`;
}

function expectBlockedWithoutDisclosure(
result: ReturnType<typeof runAction>,
) {
const combined = `${result.stdout}\n${result.stderr}`;
expect(result.status).not.toBe(0);
expect(result.outputs.committed).toBe('false');
expect(combined).toMatch(/not allowed/);
expect(combined).not.toContain(MARKER_A);
expect(combined).not.toContain(MARKER_B);
}

it('rejects add with pathspec_error_handling exitImmediately', () => {
const f = fixture!;
const dummyPath = writeDummyOutsideClone(f);
const before = gitRevParse(f.local, 'HEAD');

const result = runAction(f, {
add: pathspecFromFileArgs(dummyPath),
pathspec_error_handling: 'exitImmediately',
push: 'false',
});

expectBlockedWithoutDisclosure(result);
expect(gitRevParse(f.local, 'HEAD')).toBe(before);
});

it('rejects add with default pathspec_error_handling ignore', () => {
const f = fixture!;
const dummyPath = writeDummyOutsideClone(f);
const before = gitRevParse(f.local, 'HEAD');

const result = runAction(f, {
add: pathspecFromFileArgs(dummyPath),
push: 'false',
});

expectBlockedWithoutDisclosure(result);
expect(gitRevParse(f.local, 'HEAD')).toBe(before);
});

it('rejects add during dry_run at parse, not git add --dry-run', () => {
const f = fixture!;
const dummyPath = writeDummyOutsideClone(f);
const before = gitRevParse(f.local, 'HEAD');

const result = runAction(f, {
add: pathspecFromFileArgs(dummyPath),
dry_run: 'true',
push: 'false',
});

expectBlockedWithoutDisclosure(result);
expect(gitRevParse(f.local, 'HEAD')).toBe(before);
const combined = `${result.stdout}\n${result.stderr}`;
expect(combined).not.toMatch(/fatal: pathspec/);
expect(combined).not.toMatch(/Dry run completed/i);
});

it('rejects remove with the same options', () => {
const f = fixture!;
const dummyPath = writeDummyOutsideClone(f);
const before = gitRevParse(f.local, 'HEAD');

const result = runAction(f, {
add: '',
remove: pathspecFromFileArgs(dummyPath),
push: 'false',
});

expectBlockedWithoutDisclosure(result);
expect(gitRevParse(f.local, 'HEAD')).toBe(before);
});

it('rejects commit with the same options', () => {
const f = fixture!;
const dummyPath = writeDummyOutsideClone(f);
writeFile(f.local, 'commit-args.txt', 'changed\n');
const before = gitRevParse(f.local, 'HEAD');

const result = runAction(f, {
commit: pathspecFromFileArgs(dummyPath),
push: 'false',
});

expectBlockedWithoutDisclosure(result);
expect(gitRevParse(f.local, 'HEAD')).toBe(before);
});

it('rejects a YAML array element with the same options', () => {
const f = fixture!;
const dummyPath = writeDummyOutsideClone(f);
const before = gitRevParse(f.local, 'HEAD');

const result = runAction(f, {
add: JSON.stringify([pathspecFromFileArgs(dummyPath)]),
push: 'false',
});

expectBlockedWithoutDisclosure(result);
expect(gitRevParse(f.local, 'HEAD')).toBe(before);
});
});
});
61 changes: 61 additions & 0 deletions test/util.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,67 @@ describe('matchGitArgs', () => {
);
});

it('rejects --pathspec-from-file inline and separate-arg forms', () => {
expect(() => matchGitArgs('--pathspec-from-file=/path')).toThrow(
/pathspecs from a file/,
);
expect(() => matchGitArgs('--pathspec-from-file /path')).toThrow(
/pathspecs from a file/,
);
});

it('rejects --pathspec-file-nul alone and combined with --pathspec-from-file', () => {
expect(() => matchGitArgs('--pathspec-file-nul')).toThrow(
/pathspecs from a file/,
);
expect(() =>
matchGitArgs('--pathspec-from-file=/path --pathspec-file-nul'),
).toThrow(/pathspecs from a file/);
});

it('rejects --pathspec-from-file and --pathspec-file-nul abbreviations', () => {
expect(() => matchGitArgs('--pathspec-fr=/path')).toThrow(
/pathspecs from a file/,
);
expect(() => matchGitArgs('--pathspec-from=/path')).toThrow(
/pathspecs from a file/,
);
expect(() => matchGitArgs('--pathspec-fi')).toThrow(
/pathspecs from a file/,
);
expect(() => matchGitArgs('--pathspec-file')).toThrow(
/pathspecs from a file/,
);
});

it('preserves -m / --message values that look like --pathspec-from-file', () => {
expect(matchGitArgs('-m "--pathspec-from-file=/x"')).toStrictEqual([
'-m',
'--pathspec-from-file=/x',
]);
expect(matchGitArgs('--message --pathspec-from-file=/x')).toStrictEqual([
'--message',
'--pathspec-from-file=/x',
]);
});

it('still rejects a real --pathspec-from-file after a message value', () => {
expect(() => matchGitArgs('-m "ok" --pathspec-from-file=/x')).toThrow(
/pathspecs from a file/,
);
});

it('still rejects --pathspec-from-file when allowUnsafeGitProtocols is true', () => {
expect(() =>
matchGitArgs('--pathspec-from-file=/path', {
allowUnsafeGitProtocols: true,
}),
).toThrow(/pathspecs from a file/);
expect(() =>
matchGitArgs('--pathspec-file-nul', {allowUnsafeGitProtocols: true}),
).toThrow(/pathspecs from a file/);
});

it('rejects scheme:: remote-helper URL tokens (PoC form)', () => {
expect(() => matchGitArgs('ext::sh -c touch\\ /tmp/pwned')).toThrow(
/remote-helper URLs/,
Expand Down