Skip to content

Commit fdd7d8b

Browse files
authored
fix(test-optimization): detect swapped directories when inode numbers exceed 2^53 (#9580)
The fail-closed filesystem guards pin a directory identity as `{ dev, ino }` read from a number-typed `fs.Stats`. Windows reports a 64-bit file reference number that packs a 16-bit sequence number above the 48-bit file index, so once a volume reuses index numbers the value passes 2^53 and two distinct directories round to the same double. The Windows tracing job therefore rejected a swapped payload directory on most runs and accepted it on others. An accepted swap lets the exporter write payloads into a directory it no longer owns, and lets generated-file cleanup delete a customer file it never created.
1 parent c0282b6 commit fdd7d8b

9 files changed

Lines changed: 164 additions & 29 deletions

File tree

ci/test-optimization-validation/command-output-policy.js

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -71,21 +71,21 @@ function cleanupCommandOutputs (states) {
7171
*
7272
* @param {string} outputPath output path
7373
* @param {string} repositoryRoot repository root
74-
* @returns {{path: string, dev: number, ino: number}[]} parent identities
74+
* @returns {{path: string, dev: bigint, ino: bigint}[]} parent identities
7575
*/
7676
function captureExistingParentIdentities (outputPath, repositoryRoot) {
7777
const identities = []
7878
const relative = path.relative(repositoryRoot, path.dirname(outputPath))
7979
let current = repositoryRoot
8080
for (const segment of relative ? relative.split(path.sep) : []) {
81-
const stat = fs.lstatSync(current)
81+
const stat = fs.lstatSync(current, { bigint: true })
8282
assertRegularDirectory(stat, current)
8383
identities.push({ path: current, dev: stat.dev, ino: stat.ino })
8484
current = path.join(current, segment)
8585
if (!pathExists(current)) return identities
8686
}
8787

88-
const stat = fs.lstatSync(current)
88+
const stat = fs.lstatSync(current, { bigint: true })
8989
assertRegularDirectory(stat, current)
9090
identities.push({ path: current, dev: stat.dev, ino: stat.ino })
9191
return identities
@@ -98,7 +98,7 @@ function captureExistingParentIdentities (outputPath, repositoryRoot) {
9898
*/
9999
function assertOutputParentsUnchanged (state) {
100100
for (const identity of state.parentIdentities) {
101-
const stat = fs.lstatSync(identity.path)
101+
const stat = fs.lstatSync(identity.path, { bigint: true })
102102
assertRegularDirectory(stat, identity.path)
103103
if (stat.dev !== identity.dev || stat.ino !== identity.ino) {
104104
throw new Error(`Refusing command output cleanup because a parent directory changed: ${identity.path}`)
@@ -118,7 +118,7 @@ function assertOutputParentsUnchanged (state) {
118118
/**
119119
* Refuses symbolic links and non-directory parent components.
120120
*
121-
* @param {fs.Stats} stat path status
121+
* @param {fs.Stats | fs.BigIntStats} stat path status
122122
* @param {string} directory directory path
123123
*/
124124
function assertRegularDirectory (stat, directory) {

ci/test-optimization-validation/generated-files.js

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
'use strict'
22

3-
const fs = require('fs')
4-
const path = require('path')
3+
const fs = require('node:fs')
4+
const path = require('node:path')
55

66
const {
77
MAX_GENERATED_FILES,
@@ -251,7 +251,7 @@ function forgetWrittenGeneratedFiles (filenames) {
251251
function authorizePathForCleanup (root, filename) {
252252
const lexicalRoot = path.resolve(root)
253253
const physicalRoot = fs.realpathSync(lexicalRoot)
254-
const rootStat = fs.statSync(physicalRoot)
254+
const rootStat = fs.statSync(physicalRoot, { bigint: true })
255255
const authorization = {
256256
lexicalRoot,
257257
physicalRoot,
@@ -276,7 +276,7 @@ function pinCleanupParent (authorization, filename) {
276276
try {
277277
const physicalParent = fs.realpathSync(path.dirname(filename))
278278
if (!isPathInside(authorization.physicalRoot, physicalParent)) return
279-
const parentStat = fs.statSync(physicalParent)
279+
const parentStat = fs.statSync(physicalParent, { bigint: true })
280280
authorization.physicalParent = physicalParent
281281
authorization.parentDevice = parentStat.dev
282282
authorization.parentInode = parentStat.ino
@@ -285,7 +285,7 @@ function pinCleanupParent (authorization, filename) {
285285

286286
function pinCleanupTarget (authorization, filename) {
287287
try {
288-
const targetStat = fs.lstatSync(filename)
288+
const targetStat = fs.lstatSync(filename, { bigint: true })
289289
authorization.targetDevice = targetStat.dev
290290
authorization.targetInode = targetStat.ino
291291
} catch {}
@@ -294,22 +294,22 @@ function pinCleanupTarget (authorization, filename) {
294294
function isCleanupAuthorizationValid (filename, authorization) {
295295
try {
296296
const currentPhysicalRoot = fs.realpathSync(authorization.lexicalRoot)
297-
const rootStat = fs.statSync(currentPhysicalRoot)
297+
const rootStat = fs.statSync(currentPhysicalRoot, { bigint: true })
298298
if (currentPhysicalRoot !== authorization.physicalRoot ||
299299
rootStat.dev !== authorization.rootDevice || rootStat.ino !== authorization.rootInode) {
300300
return false
301301
}
302302

303303
if (authorization.physicalParent === undefined) return false
304304
const physicalParent = fs.realpathSync(path.dirname(filename))
305-
const parentStat = fs.statSync(physicalParent)
305+
const parentStat = fs.statSync(physicalParent, { bigint: true })
306306
if (physicalParent !== authorization.physicalParent ||
307307
parentStat.dev !== authorization.parentDevice || parentStat.ino !== authorization.parentInode) {
308308
return false
309309
}
310310

311311
if (authorization.targetDevice !== undefined) {
312-
const targetStat = fs.lstatSync(filename)
312+
const targetStat = fs.lstatSync(filename, { bigint: true })
313313
if (targetStat.dev !== authorization.targetDevice || targetStat.ino !== authorization.targetInode) {
314314
return false
315315
}

ci/test-optimization-validation/offline-output.js

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,8 @@ function readOfflineOutput (outputRoot) {
5757
if (!exporterInitialized) return emptyOutput()
5858

5959
const state = {
60-
bytes: 0,
61-
completionBytes: 0,
60+
bytes: 0n,
61+
completionBytes: 0n,
6262
decodedEntries: 0,
6363
files: 0,
6464
}
@@ -150,7 +150,7 @@ function readPayloadFiles (payloadsRoot, kind, state, consume) {
150150
}
151151

152152
function readRegularFile (filename, individualLimit, state, completion) {
153-
const stat = fs.lstatSync(filename)
153+
const stat = fs.lstatSync(filename, { bigint: true })
154154
if (!stat.isFile() || stat.isSymbolicLink() || stat.nlink > 1) {
155155
throw new Error('Offline validation artifact must be a regular, unlinked file.')
156156
}
@@ -163,16 +163,16 @@ function readRegularFile (filename, individualLimit, state, completion) {
163163

164164
const file = fs.openSync(filename, fs.constants.O_RDONLY | (fs.constants.O_NOFOLLOW || 0))
165165
try {
166-
const opened = fs.fstatSync(file)
166+
const opened = fs.fstatSync(file, { bigint: true })
167167
if (!opened.isFile() || opened.nlink > 1 || opened.dev !== stat.dev || opened.ino !== stat.ino) {
168168
throw new Error('Offline validation artifact changed while it was opened.')
169169
}
170170
const buffer = fs.readFileSync(file)
171-
const completed = fs.fstatSync(file)
171+
const completed = fs.fstatSync(file, { bigint: true })
172172
if (completed.size !== opened.size || completed.mtimeMs !== opened.mtimeMs) {
173173
throw new Error('Offline validation artifact changed while it was read.')
174174
}
175-
state[totalKey] += buffer.length
175+
state[totalKey] += BigInt(buffer.length)
176176
return buffer
177177
} finally {
178178
fs.closeSync(file)

ci/test-optimization-validation/safe-files.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -129,10 +129,10 @@ function openAndWrite (root, filename, data, label, creationFlag) {
129129
*
130130
* @param {string} directory directory path
131131
* @param {string} label customer-facing path label
132-
* @returns {{dev: number, ino: number}} directory identity
132+
* @returns {{dev: bigint, ino: bigint}} directory identity
133133
*/
134134
function getDirectoryIdentity (directory, label) {
135-
const stat = fs.lstatSync(directory)
135+
const stat = fs.lstatSync(directory, { bigint: true })
136136
if (!stat.isDirectory() || stat.isSymbolicLink()) {
137137
throw new Error(`Refusing ${label} because its parent is not a regular directory: ${directory}`)
138138
}
@@ -143,7 +143,7 @@ function getDirectoryIdentity (directory, label) {
143143
* Refuses replacement of a parent directory between safe creation and publication.
144144
*
145145
* @param {string} directory directory path
146-
* @param {{dev: number, ino: number}} expected expected identity
146+
* @param {{dev: bigint, ino: bigint}} expected expected identity
147147
* @param {string} label customer-facing path label
148148
*/
149149
function assertDirectoryIdentity (directory, expected, label) {
@@ -158,7 +158,7 @@ function assertDirectoryIdentity (directory, expected, label) {
158158
*
159159
* @param {string} filename temporary filename
160160
* @param {string} parent expected parent directory
161-
* @param {{dev: number, ino: number}} parentIdentity expected parent identity
161+
* @param {{dev: bigint, ino: bigint}} parentIdentity expected parent identity
162162
*/
163163
function removeTemporaryFile (filename, parent, parentIdentity) {
164164
try {

packages/dd-trace/src/ci-visibility/exporters/ci-validation/sink.js

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -303,10 +303,11 @@ function writeNewFile (filename, payload) {
303303
*
304304
* @param {string} directory directory path
305305
* @param {string} label directory label
306-
* @returns {{dev: number, ino: number}} stable directory identity
306+
* @returns {{dev: bigint, ino: bigint}} stable directory identity
307307
*/
308308
function captureDirectory (directory, label) {
309-
const stat = fs.lstatSync(directory)
309+
// Windows file reference numbers exceed 2^53, where distinct directories round to one number.
310+
const stat = fs.lstatSync(directory, { bigint: true })
310311
if (!stat.isDirectory() || stat.isSymbolicLink()) {
311312
throw new Error(`Offline Test Optimization validation ${label} must be a regular directory.`)
312313
}
@@ -317,10 +318,10 @@ function captureDirectory (directory, label) {
317318
* Creates or validates one child directory without accepting symbolic links.
318319
*
319320
* @param {string} parent parent directory path
320-
* @param {{dev: number, ino: number}} parentIdentity expected parent identity
321+
* @param {{dev: bigint, ino: bigint}} parentIdentity expected parent identity
321322
* @param {string} directory child directory path
322323
* @param {string} label directory label
323-
* @returns {{dev: number, ino: number}} stable child identity
324+
* @returns {{dev: bigint, ino: bigint}} stable child identity
324325
*/
325326
function createDirectory (parent, parentIdentity, directory, label) {
326327
assertDirectoryUnchanged(parent, parentIdentity, 'parent output')
@@ -336,7 +337,7 @@ function createDirectory (parent, parentIdentity, directory, label) {
336337
* Rejects a directory that changed after sink construction.
337338
*
338339
* @param {string} directory directory path
339-
* @param {{dev: number, ino: number}} identity expected directory identity
340+
* @param {{dev: bigint, ino: bigint}} identity expected directory identity
340341
* @param {string} label directory label
341342
*/
342343
function assertDirectoryUnchanged (directory, identity, label) {
@@ -351,7 +352,7 @@ function assertDirectoryUnchanged (directory, identity, label) {
351352
*
352353
* @param {string} filename partial payload path
353354
* @param {string} directory expected parent directory
354-
* @param {{dev: number, ino: number}} identity expected parent identity
355+
* @param {{dev: bigint, ino: bigint}} identity expected parent identity
355356
*/
356357
function removePartialFile (filename, directory, identity) {
357358
try {

packages/dd-trace/test/ci-visibility/exporters/ci-validation.spec.js

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ const path = require('node:path')
1111

1212
const msgpack = require('@msgpack/msgpack')
1313
const { afterEach, beforeEach, describe, it } = require('mocha')
14+
const proxyquire = require('proxyquire')
1415
const sinon = require('sinon')
1516

1617
require('../../setup/core')
@@ -26,6 +27,7 @@ const { CiValidationSink, MAX_OUTPUT_FILES, SUMMARY_PREFIX } =
2627
require('../../../src/ci-visibility/exporters/ci-validation/sink')
2728
const CiValidationWriter = require('../../../src/ci-visibility/exporters/ci-validation/writer')
2829
const CiValidationExporter = require('../../../src/ci-visibility/exporters/ci-validation')
30+
const { createWindowsFileReferenceFs } = require('../validation-test-helpers')
2931

3032
const VALIDATION_MANIFEST_ENV = '_DD_TEST_OPTIMIZATION_VALIDATION_MANIFEST_FILE'
3133
const VALIDATION_OUTPUT_ENV = '_DD_TEST_OPTIMIZATION_VALIDATION_OUTPUT_DIR'
@@ -139,6 +141,28 @@ describe('CI validation offline output', () => {
139141
assert.deepStrictEqual(summary.errors, ['output_write_failed'])
140142
})
141143

144+
it('fails closed when a swapped payload directory reuses a file reference above 2^53', () => {
145+
const { CiValidationSink: WindowsCiValidationSink } =
146+
proxyquire('../../../src/ci-visibility/exporters/ci-validation/sink', {
147+
'node:fs': createWindowsFileReferenceFs(),
148+
})
149+
150+
const sink = new WindowsCiValidationSink(outputRoot)
151+
const writer = new CiValidationWriter({ sink, tags: {} })
152+
const testsDirectory = path.join(outputRoot, 'payloads', 'tests')
153+
fs.renameSync(testsDirectory, `${testsDirectory}-original`)
154+
fs.mkdirSync(testsDirectory)
155+
156+
writer.append([createTestSpan()])
157+
writer.flush()
158+
sink.writeSummary()
159+
160+
assert.strictEqual(getPayloadFiles(outputRoot, 'tests').length, 0)
161+
assert.strictEqual(process.exitCode, 1)
162+
const summary = JSON.parse(stderrWrite.firstCall.args[0].slice(SUMMARY_PREFIX.length))
163+
assert.deepStrictEqual(summary.errors, ['output_write_failed'])
164+
})
165+
142166
it('reports bounded cache input results in one summary', () => {
143167
const sink = new CiValidationSink(outputRoot)
144168

packages/dd-trace/test/ci-visibility/generated-files.spec.js

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,14 @@ const fs = require('node:fs')
55
const os = require('node:os')
66
const path = require('node:path')
77

8+
const proxyquire = require('proxyquire')
9+
810
const {
911
cleanupGeneratedFiles,
1012
cleanupGeneratedRuntimeFiles,
1113
writeGeneratedFiles,
1214
} = require('../../../../ci/test-optimization-validation/generated-files')
15+
const { createWindowsFileReferenceFs } = require('./validation-test-helpers')
1316

1417
describe('test optimization validation generated files', () => {
1518
it('allows existing generated files when the content matches', () => {
@@ -274,6 +277,30 @@ describe('test optimization validation generated files', () => {
274277
}
275278
})
276279

280+
it('retains a replaced generated file that reuses a file reference above 2^53', () => {
281+
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'dd-generated-files-windows-'))
282+
const filename = path.join(root, 'dd-test-optimization-validation.test.js')
283+
const original = path.join(root, 'original-generated.test.js')
284+
const framework = getFramework(root, filename)
285+
const windowsGeneratedFiles = proxyquire('../../../../ci/test-optimization-validation/generated-files', {
286+
'node:fs': createWindowsFileReferenceFs(),
287+
})
288+
289+
try {
290+
windowsGeneratedFiles.writeGeneratedFiles(framework)
291+
fs.renameSync(filename, original)
292+
fs.writeFileSync(filename, 'customer replacement\n')
293+
294+
const cleanup = windowsGeneratedFiles.cleanupGeneratedFiles({ frameworks: [framework] })
295+
296+
assert.strictEqual(cleanup.status, 'incomplete')
297+
assert.strictEqual(cleanup.filesRetained, 1)
298+
assert.strictEqual(fs.readFileSync(filename, 'utf8'), 'customer replacement\n')
299+
} finally {
300+
fs.rmSync(root, { recursive: true, force: true })
301+
}
302+
})
303+
277304
it('reports temporary files intentionally retained by the approved plan', () => {
278305
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'dd-generated-files-kept-'))
279306
const filename = path.join(root, 'dd-test-optimization-validation.test.js')

packages/dd-trace/test/ci-visibility/validation-execution-phases.spec.js

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ const { getBasicCommand } = require('../../../../ci/test-optimization-validation
2525
const {
2626
createLoadedManifest,
2727
createRepositoryFixture,
28+
createWindowsFileReferenceFs,
2829
removeFixture,
2930
} = require('./validation-test-helpers')
3031

@@ -404,6 +405,51 @@ describe('test optimization validation execution boundary', () => {
404405
assert.match(withCiPreloads('', framework).replaceAll('\\', '/'), /-r "?[^"]*\/ci\/init\.js"?$/)
405406
})
406407

408+
it('refuses command output cleanup when a parent is swapped and reuses a file reference above 2^53', () => {
409+
const { cleanupCommandOutputs, prepareCommandOutputs } =
410+
proxyquire('../../../../ci/test-optimization-validation/command-output-policy', {
411+
'node:fs': createWindowsFileReferenceFs(),
412+
})
413+
const outputParent = path.join(fixture.root, 'command-output')
414+
fs.mkdirSync(outputParent)
415+
const states = prepareCommandOutputs({
416+
artifactRoot: out,
417+
command: { cwd: fixture.root, outputPaths: [path.join(outputParent, 'result.json')] },
418+
repositoryRoot: fixture.root,
419+
})
420+
421+
fs.renameSync(outputParent, `${outputParent}-original`)
422+
fs.mkdirSync(outputParent)
423+
424+
assert.throws(() => cleanupCommandOutputs(states), /parent directory changed/)
425+
})
426+
427+
it('refuses to publish a report when its parent is swapped and reuses a file reference above 2^53', () => {
428+
const parent = path.join(fixture.root, 'safe-write')
429+
fs.mkdirSync(parent)
430+
let swapped = false
431+
const { writeFileSafely } = proxyquire('../../../../ci/test-optimization-validation/safe-files', {
432+
'node:fs': createWindowsFileReferenceFs({
433+
// Windows refuses to rename a directory that still holds an open handle, so the swap waits
434+
// until the temporary file is closed.
435+
closeSync: (file) => {
436+
fs.closeSync(file)
437+
if (!swapped) {
438+
swapped = true
439+
fs.renameSync(parent, `${parent}-original`)
440+
fs.mkdirSync(parent)
441+
}
442+
},
443+
}),
444+
})
445+
446+
assert.throws(
447+
() => writeFileSafely(fixture.root, path.join(parent, 'report.json'), '{}', 'validation report'),
448+
/parent directory changed during the write/
449+
)
450+
assert.strictEqual(swapped, true)
451+
})
452+
407453
/**
408454
* Executes a direct command with standard test artifacts.
409455
*

0 commit comments

Comments
 (0)