Skip to content

Commit 8305724

Browse files
committed
harden buildx scoped config path handling
Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
1 parent 371161b commit 8305724

2 files changed

Lines changed: 107 additions & 6 deletions

File tree

__tests__/context.test.ts

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,87 @@ test('getAuthList uses the default Docker Hub registry when computing scoped con
3535
});
3636
});
3737

38+
test('getAuthList supports @ scopes appended to the registry config dir', async () => {
39+
process.env['INPUT_USERNAME'] = 'dbowie';
40+
process.env['INPUT_PASSWORD'] = 'groundcontrol';
41+
process.env['INPUT_SCOPE'] = '@push';
42+
process.env['INPUT_LOGOUT'] = 'false';
43+
const [auth] = getAuthList(getInputs());
44+
expect(auth).toMatchObject({
45+
configDir: path.join(Buildx.configDir, 'config', 'registry-1.docker.io') + '@push'
46+
});
47+
});
48+
49+
test('getAuthList supports repository scopes with appended actions', async () => {
50+
process.env['INPUT_USERNAME'] = 'dbowie';
51+
process.env['INPUT_PASSWORD'] = 'groundcontrol';
52+
process.env['INPUT_SCOPE'] = 'docker/buildx-bin@push';
53+
process.env['INPUT_LOGOUT'] = 'false';
54+
const [auth] = getAuthList(getInputs());
55+
expect(auth).toMatchObject({
56+
configDir: path.join(Buildx.configDir, 'config', 'registry-1.docker.io', 'docker', 'buildx-bin@push')
57+
});
58+
});
59+
60+
test('getAuthList supports comma-separated scope actions', async () => {
61+
process.env['INPUT_USERNAME'] = 'dbowie';
62+
process.env['INPUT_PASSWORD'] = 'groundcontrol';
63+
process.env['INPUT_SCOPE'] = 'docker/buildx-bin@pull,push';
64+
process.env['INPUT_LOGOUT'] = 'false';
65+
const [auth] = getAuthList(getInputs());
66+
expect(auth).toMatchObject({
67+
configDir: path.join(Buildx.configDir, 'config', 'registry-1.docker.io', 'docker', 'buildx-bin@pull,push')
68+
});
69+
});
70+
71+
// prettier-ignore
72+
test.each([
73+
'../../../../work/leaked',
74+
'..',
75+
'foo/../../../../etc',
76+
'@../../../leaked',
77+
'foo/bar@../../../leaked',
78+
'@push@pull',
79+
'foo/bar@push@pull',
80+
'@push,',
81+
'@,push',
82+
'@pull,,push',
83+
'@Push',
84+
path.join(path.parse(Buildx.configDir).root, 'work', 'leaked')
85+
])('getAuthList rejects unsafe or unsupported scope path: %s', async scope => {
86+
expect(() => {
87+
getAuthList({
88+
registry: '',
89+
username: 'dbowie',
90+
password: 'groundcontrol',
91+
scope,
92+
ecr: '',
93+
logout: false,
94+
registryAuth: ''
95+
});
96+
}).toThrow(/Invalid scope/);
97+
});
98+
99+
// prettier-ignore
100+
test.each([
101+
'../../../../work/leaked',
102+
'..',
103+
'foo/../../../../etc',
104+
path.join(path.parse(Buildx.configDir).root, 'work', 'leaked')
105+
])('getAuthList rejects unsafe registry path: %s', async registry => {
106+
expect(() => {
107+
getAuthList({
108+
registry,
109+
username: 'dbowie',
110+
password: 'groundcontrol',
111+
scope: '@push',
112+
ecr: '',
113+
logout: false,
114+
registryAuth: ''
115+
});
116+
}).toThrow(/Invalid registry/);
117+
});
118+
38119
test('getAuthList skips secret masking when registry-auth password is absent', async () => {
39120
const stdoutWriteSpy = vi.spyOn(process.stdout, 'write').mockImplementation(() => true);
40121
const [auth] = getAuthList({

src/context.ts

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -77,13 +77,33 @@ export function scopeToConfigDir(registry: string, scope?: string): string {
7777
if (scopeDisabled() || !scope || scope === '') {
7878
return '';
7979
}
80-
let configDir = path.join(Buildx.configDir, 'config', registry === 'docker.io' ? 'registry-1.docker.io' : registry);
81-
if (scope.startsWith('@')) {
82-
configDir += scope;
83-
} else {
84-
configDir = path.join(configDir, scope);
80+
const configRoot = path.resolve(Buildx.configDir, 'config');
81+
const registryDir = path.resolve(configRoot, registry === 'docker.io' ? 'registry-1.docker.io' : registry);
82+
if (!isChildPath(configRoot, registryDir)) {
83+
throw new Error(`Invalid registry '${registry}': resolved config path escapes the Buildx config directory`);
84+
}
85+
const scopeParts = scope.split('@');
86+
if (scopeParts.length > 2) {
87+
throw new Error(`Invalid scope '${scope}': scope can contain at most one @ separator`);
88+
}
89+
const [scopePath, scopeActions] = scopeParts;
90+
if (scopeActions !== undefined && !/^[a-z]+(,[a-z]+)*$/.test(scopeActions)) {
91+
throw new Error(`Invalid scope '${scope}': scope actions must be lowercase names separated by commas`);
92+
}
93+
const scopeSuffix = scopeActions === undefined ? '' : `@${scopeActions}`;
94+
if (scopePath === '') {
95+
return `${registryDir}${scopeSuffix}`;
8596
}
86-
return configDir;
97+
const configDir = path.resolve(registryDir, scopePath);
98+
if (!isChildPath(registryDir, configDir)) {
99+
throw new Error(`Invalid scope '${scope}': resolved config path escapes the Buildx config directory`);
100+
}
101+
return `${configDir}${scopeSuffix}`;
102+
}
103+
104+
function isChildPath(parent: string, child: string): boolean {
105+
const relativePath = path.relative(parent, child);
106+
return relativePath !== '' && relativePath !== '..' && !relativePath.startsWith(`..${path.sep}`) && !path.isAbsolute(relativePath);
87107
}
88108

89109
function scopeDisabled(): boolean {

0 commit comments

Comments
 (0)