Skip to content

Commit f4f54b6

Browse files
committed
fix(storage): close snapshot policy review gaps
Generated-by: OpenAI Codex
1 parent ee32f8b commit f4f54b6

3 files changed

Lines changed: 165 additions & 30 deletions

File tree

packages/storage/src/__tests__/quiescent-session-snapshot.test.ts

Lines changed: 90 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,25 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
120
import assert from 'node:assert/strict';
221
import {
22+
copyFile,
323
mkdir,
424
mkdtemp,
525
readFile,
@@ -31,8 +51,8 @@ afterEach(async () => {
3151

3252
test('prepares state and workspace under one quiescence boundary, then releases live writers', async () => {
3353
const fixture = await createFixture();
34-
let liveState = 'state-at-boundary';
35-
let liveWorkspace = 'workspace-at-boundary';
54+
await writeFile(join(fixture.liveStateRoot, 'runtime.sqlite'), 'state-at-boundary', 'utf8');
55+
await writeFile(join(fixture.liveWorkspaceRoot, 'main.ts'), 'workspace-at-boundary', 'utf8');
3656
const events: string[] = [];
3757
let quiescent = false;
3858

@@ -57,7 +77,10 @@ test('prepares state and workspace under one quiescence boundary, then releases
5777
assert.equal(quiescent, true);
5878
events.push('state');
5979
await mkdir(input.destinationRoot);
60-
await writeFile(join(input.destinationRoot, 'runtime.sqlite'), liveState, 'utf8');
80+
await copyFile(
81+
join(fixture.liveStateRoot, 'runtime.sqlite'),
82+
join(input.destinationRoot, 'runtime.sqlite'),
83+
);
6184
return {
6285
mediaType: 'application/vnd.maka.session-state-identity+json;version=1',
6386
bytes: Buffer.from('{"makaSessionId":"session-1"}', 'utf8'),
@@ -70,7 +93,10 @@ test('prepares state and workspace under one quiescence boundary, then releases
7093
assert.equal(input.policy, SESSION_SNAPSHOT_WORKSPACE_POLICY_V1);
7194
events.push('workspace');
7295
await mkdir(input.destinationRoot);
73-
await writeFile(join(input.destinationRoot, 'main.ts'), liveWorkspace, 'utf8');
96+
await copyFile(
97+
join(fixture.liveWorkspaceRoot, 'main.ts'),
98+
join(input.destinationRoot, 'main.ts'),
99+
);
74100
return workspaceResult({ includedEntries: 1 });
75101
},
76102
},
@@ -82,15 +108,15 @@ test('prepares state and workspace under one quiescence boundary, then releases
82108
assert.notEqual(handle.snapshot.workspaceRoot, fixture.liveWorkspaceRoot);
83109
assert.equal(
84110
await readFile(join(handle.snapshot.stateRoot, 'runtime.sqlite'), 'utf8'),
85-
liveState,
111+
'state-at-boundary',
86112
);
87113
assert.equal(
88114
await readFile(join(handle.snapshot.workspaceRoot, 'main.ts'), 'utf8'),
89-
liveWorkspace,
115+
'workspace-at-boundary',
90116
);
91117

92-
liveState = 'later-state';
93-
liveWorkspace = 'later-workspace';
118+
await writeFile(join(fixture.liveStateRoot, 'runtime.sqlite'), 'later-state', 'utf8');
119+
await writeFile(join(fixture.liveWorkspaceRoot, 'main.ts'), 'later-workspace', 'utf8');
94120
assert.equal(
95121
await readFile(join(handle.snapshot.stateRoot, 'runtime.sqlite'), 'utf8'),
96122
'state-at-boundary',
@@ -308,6 +334,41 @@ test('cancellation and an expired deadline stop before staging begins', async ()
308334
assert.deepEqual(await readdir(fixture.stagingParent), []);
309335
});
310336

337+
test('a deadline beyond the Node timer limit is rescheduled until the absolute time', async (t) => {
338+
t.mock.timers.enable({ apis: ['Date', 'setTimeout'], now: 0 });
339+
const fixture = await createFixture();
340+
const authorityEntered = deferred<void>();
341+
let cancellationSignal: AbortSignal | undefined;
342+
const coordinator = createFileQuiescentSessionSnapshotCoordinator({
343+
stagingParent: fixture.stagingParent,
344+
privateStagingRootAuthority,
345+
now: Date.now,
346+
quiescence: {
347+
async runQuiescent(input) {
348+
cancellationSignal = input.cancellation.signal;
349+
authorityEntered.resolve();
350+
await waitForAbort(input.cancellation);
351+
throw Object.assign(new Error('aborted'), { name: 'AbortError' });
352+
},
353+
},
354+
state: directoryStatePreparer,
355+
workspace: directoryWorkspacePreparer,
356+
});
357+
const timerLimit = 2_147_483_647;
358+
const preparation = coordinator.prepare({
359+
makaSessionId: 'long-deadline',
360+
deadlineAt: timerLimit + 1_000,
361+
});
362+
await authorityEntered.promise;
363+
364+
t.mock.timers.tick(timerLimit);
365+
assert.equal(cancellationSignal?.aborted, false);
366+
t.mock.timers.tick(999);
367+
assert.equal(cancellationSignal?.aborted, false);
368+
t.mock.timers.tick(1);
369+
await assert.rejects(preparation, isSnapshotError('snapshot_cancelled'));
370+
});
371+
311372
test('cancellation while waiting for quiescence is propagated as snapshot_cancelled', async () => {
312373
const fixture = await createFixture();
313374
const authorityEntered = deferred<void>();
@@ -547,12 +608,21 @@ test('V1 workspace policy includes portable inputs, excludes rebuildable data, a
547608
['.turbo/cache.bin', 'file', { kind: 'exclude', category: 'cache' }],
548609
['logs/agent.txt', 'file', { kind: 'exclude', category: 'log' }],
549610
['debug.log', 'file', { kind: 'exclude', category: 'log' }],
550-
['secrets.log', 'file', { kind: 'exclude', category: 'log' }],
611+
['secrets.log', 'file', { kind: 'reject', category: 'known_secret_file' }],
612+
['.env.log', 'file', { kind: 'reject', category: 'known_secret_file' }],
613+
['keys/private-key.log', 'file', { kind: 'reject', category: 'known_secret_file' }],
551614
['.maka-runtime/input.json', 'file', { kind: 'exclude', category: 'runtime_scratch' }],
552615
['.env.local', 'file', { kind: 'reject', category: 'known_secret_file' }],
616+
['.env.example', 'file', { kind: 'include' }],
617+
['.env.template', 'file', { kind: 'include' }],
618+
['.env.example.local', 'file', { kind: 'reject', category: 'known_secret_file' }],
553619
['keys/id_ed25519', 'file', { kind: 'reject', category: 'known_secret_file' }],
620+
['keys/id_ed25519.pub', 'file', { kind: 'include' }],
621+
['keys/id_rsa.pub', 'file', { kind: 'include' }],
554622
['credentials.yaml', 'file', { kind: 'reject', category: 'known_secret_file' }],
555623
['secrets.json', 'file', { kind: 'reject', category: 'known_secret_file' }],
624+
['src/secrets.ts', 'file', { kind: 'include' }],
625+
['docs/secrets.md', 'file', { kind: 'include' }],
556626
['.terraformrc', 'file', { kind: 'reject', category: 'known_secret_file' }],
557627
['.git-credentials.lock', 'file', { kind: 'reject', category: 'known_secret_file' }],
558628
['keys/client-private-key.pem', 'file', { kind: 'reject', category: 'known_secret_file' }],
@@ -562,7 +632,13 @@ test('V1 workspace policy includes portable inputs, excludes rebuildable data, a
562632
['certs/client.csr', 'file', { kind: 'include' }],
563633
['certs/client.pem', 'file', { kind: 'include' }],
564634
['certs/client.der', 'file', { kind: 'include' }],
635+
['privkey.pem', 'file', { kind: 'reject', category: 'known_secret_file' }],
636+
['private.pem', 'file', { kind: 'reject', category: 'known_secret_file' }],
565637
['keys/service-account.json', 'file', { kind: 'reject', category: 'known_secret_file' }],
638+
['secrets', 'directory', { kind: 'reject', category: 'known_secret_file' }],
639+
['secrets/token', 'file', { kind: 'reject', category: 'known_secret_file' }],
640+
['credentials/oauth.json', 'file', { kind: 'reject', category: 'known_secret_file' }],
641+
['private/token', 'file', { kind: 'reject', category: 'known_secret_file' }],
566642
['.ssh/config', 'file', { kind: 'reject', category: 'known_secret_file' }],
567643
['.aws/credentials', 'file', { kind: 'reject', category: 'known_secret_file' }],
568644
['.cargo/credentials', 'file', { kind: 'reject', category: 'known_secret_file' }],
@@ -575,6 +651,11 @@ test('V1 workspace policy includes portable inputs, excludes rebuildable data, a
575651
],
576652
['../escape', 'file', { kind: 'reject', category: 'unsafe_path' }],
577653
['a\\b', 'file', { kind: 'reject', category: 'unsafe_path' }],
654+
['CON', 'file', { kind: 'reject', category: 'unsafe_path' }],
655+
['nested/LPT1.txt', 'file', { kind: 'reject', category: 'unsafe_path' }],
656+
['foo:bar', 'file', { kind: 'reject', category: 'unsafe_path' }],
657+
['name.', 'file', { kind: 'reject', category: 'unsafe_path' }],
658+
['name ', 'directory', { kind: 'reject', category: 'unsafe_path' }],
578659
] as const;
579660

580661
for (const [relativePath, kind, expected] of cases) {

packages/storage/src/quiescent-session-snapshot.ts

Lines changed: 64 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,22 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
120
import { randomUUID } from 'node:crypto';
221
import { constants as fsConstants } from 'node:fs';
322
import { lstat, mkdir, open, realpath, rename, rm } from 'node:fs/promises';
@@ -7,6 +26,7 @@ import type {
726
OpaqueStateIdentityDescriptor,
827
PreparedSessionBundleSnapshot,
928
} from './session-bundle-contract.js';
29+
import { isSessionBundleUstarPathV1 } from './session-bundle-ustar.js';
1030
import { isSafeSessionId } from './session-store.js';
1131

1232
export const SESSION_SNAPSHOT_POLICY_VERSION = 1 as const;
@@ -15,6 +35,7 @@ export const SESSION_SNAPSHOT_STAGING_SCHEMA_VERSION = 1 as const;
1535
const SNAPSHOT_ID_PATTERN =
1636
/^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/u;
1737
const MAX_OWNER_RECORD_BYTES = 1_024;
38+
const MAX_TIMER_DELAY_MS = 2_147_483_647;
1839
const NO_FOLLOW_OPEN_FLAG = process.platform === 'win32' ? 0 : fsConstants.O_NOFOLLOW;
1940

2041
export type SessionSnapshotWorkspaceEntryKind = 'file' | 'directory';
@@ -63,12 +84,20 @@ const INCLUDE = Object.freeze({ kind: 'include' } as const);
6384
// measurement rules introduced by #1353. Snapshot rejection is reserved for
6485
// names that identify known secret material; public certificate encodings and
6586
// other ambiguous formats are not rejected by extension alone.
87+
const PUBLIC_ENV_TEMPLATE_PATTERN = /^\.env\.(?:example|sample|template)$/i;
88+
const KNOWN_SECRET_WORKSPACE_DIRECTORY_NAMES = new Set([
89+
'.ssh',
90+
'credentials',
91+
'private',
92+
'secrets',
93+
]);
6694
const KNOWN_SECRET_WORKSPACE_FILE_PATTERNS = [
6795
/^\.env(?:\..*)?$/i,
6896
/^\.(?:npmrc|netrc|pypirc|terraformrc)$/i,
6997
/^\.git-credentials(?:\.lock)?$/i,
70-
/^(?:credentials?|secrets?)(?:\..*)?$/i,
98+
/^(?:credentials?|secrets?)(?:\.(?:cfg|conf|ini|json|log|properties|toml|ya?ml))?$/i,
7199
/(?:^|[-_.])(?:id_(?:rsa|dsa|ecdsa|ed25519)|private[-_.]?key)(?:$|[-_.])/i,
100+
/^(?:private|privkey)\.pem$/i,
72101
/\.(?:key|p12|pfx)$/i,
73102
] as const;
74103

@@ -101,19 +130,19 @@ export const SESSION_SNAPSHOT_WORKSPACE_POLICY_V1: SessionSnapshotWorkspacePolic
101130
) {
102131
return { kind: 'exclude', category: 'cache' };
103132
}
133+
if (lowerSegments.includes('.maka-runtime') || lowerSegments.includes('.maka-activation')) {
134+
return { kind: 'exclude', category: 'runtime_scratch' };
135+
}
136+
if (isKnownSecretEntry(entry.kind, lowerSegments, lowerName)) {
137+
return { kind: 'reject', category: 'known_secret_file' };
138+
}
104139
if (
105140
lowerSegments.includes('logs') ||
106141
lowerSegments.includes('.logs') ||
107142
(entry.kind === 'file' && lowerName.endsWith('.log'))
108143
) {
109144
return { kind: 'exclude', category: 'log' };
110145
}
111-
if (lowerSegments.includes('.maka-runtime') || lowerSegments.includes('.maka-activation')) {
112-
return { kind: 'exclude', category: 'runtime_scratch' };
113-
}
114-
if (entry.kind === 'file' && isKnownSecretPath(lowerSegments, lowerName)) {
115-
return { kind: 'reject', category: 'known_secret_file' };
116-
}
117146
return INCLUDE;
118147
},
119148
});
@@ -371,7 +400,7 @@ class FileQuiescentSessionSnapshotCoordinator implements QuiescentSessionSnapsho
371400
prepared = handle;
372401
return handle;
373402
} catch (error) {
374-
await cleanupAfterPreparationFailure(staging, error);
403+
return cleanupAfterPreparationFailure(staging, error);
375404
}
376405
},
377406
);
@@ -649,22 +678,29 @@ function decodeWorkspaceEntry(
649678
if (segments.some((segment) => segment.length === 0 || segment === '.' || segment === '..')) {
650679
return { kind: 'reject', category: 'unsafe_path' };
651680
}
681+
const bundlePath = `workspace/${entry.relativePath}${entry.kind === 'directory' ? '/' : ''}`;
682+
if (!isSessionBundleUstarPathV1(bundlePath)) {
683+
return { kind: 'reject', category: 'unsafe_path' };
684+
}
652685
return { kind: 'valid', segments, basename: segments.at(-1)! };
653686
}
654687

655-
function isKnownSecretPath(lowerSegments: readonly string[], lowerName: string): boolean {
688+
function isKnownSecretEntry(
689+
kind: SessionSnapshotWorkspaceEntryKind,
690+
lowerSegments: readonly string[],
691+
lowerName: string,
692+
): boolean {
693+
if (lowerSegments.some((segment) => KNOWN_SECRET_WORKSPACE_DIRECTORY_NAMES.has(segment))) {
694+
return true;
695+
}
696+
if (kind === 'directory') return false;
697+
if (PUBLIC_ENV_TEMPLATE_PATTERN.test(lowerName) || lowerName.endsWith('.pub')) return false;
656698
if (KNOWN_SECRET_WORKSPACE_FILE_PATTERNS.some((pattern) => pattern.test(lowerName))) return true;
657-
if (
658-
lowerSegments.includes('.ssh') ||
659-
lowerName === 'service-account.json' ||
660-
lowerName === 'service-account-key.json'
661-
) {
699+
if (lowerName === 'service-account.json' || lowerName === 'service-account-key.json') {
662700
return true;
663701
}
664702
return (
665703
(lowerSegments.at(-2) === '.docker' && lowerName === 'config.json') ||
666-
(lowerSegments.at(-2) === '.aws' && lowerName === 'credentials') ||
667-
(lowerSegments.at(-2) === '.cargo' && lowerName === 'credentials') ||
668704
(lowerSegments.at(-2) === '.kube' && lowerName === 'config') ||
669705
(lowerSegments.at(-2) === 'gcloud' &&
670706
lowerSegments.at(-3) === '.config' &&
@@ -785,12 +821,19 @@ function createCancellation(
785821
const controller = new AbortController();
786822
const abort = () => controller.abort();
787823
input.signal?.addEventListener('abort', abort, { once: true });
824+
let timeout: NodeJS.Timeout | undefined;
825+
const scheduleDeadline = () => {
826+
if (input.deadlineAt === undefined) return;
827+
const remaining = input.deadlineAt - now();
828+
if (remaining <= 0) {
829+
abort();
830+
return;
831+
}
832+
timeout = setTimeout(scheduleDeadline, Math.min(remaining, MAX_TIMER_DELAY_MS));
833+
timeout.unref();
834+
};
788835
const remaining = input.deadlineAt === undefined ? undefined : input.deadlineAt - now();
789-
const timeout =
790-
remaining === undefined || remaining <= 0
791-
? undefined
792-
: setTimeout(abort, Math.min(remaining, 2_147_483_647));
793-
timeout?.unref();
836+
if (remaining !== undefined && remaining > 0) scheduleDeadline();
794837
if (input.signal?.aborted || (remaining !== undefined && remaining <= 0)) abort();
795838
const value = Object.freeze({
796839
signal: controller.signal,

packages/storage/src/session-bundle-ustar.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,17 @@ export interface SessionBundleUstarHeader {
3838
size: number;
3939
}
4040

41+
/** Return whether a path belongs to the exact portable path language used by Bundle V1. */
42+
export function isSessionBundleUstarPathV1(path: string): boolean {
43+
try {
44+
splitUstarPath(path);
45+
return true;
46+
} catch (error) {
47+
if (error instanceof SessionBundleFileError && error.code === 'unsafe_path') return false;
48+
throw error;
49+
}
50+
}
51+
4152
/**
4253
* Encode the exact POSIX USTAR header admitted by Session Bundle codec V1.
4354
* Paths use a portable subset on every host: Windows device names, forbidden

0 commit comments

Comments
 (0)