Compare commits
2
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
 Gergo Magyar
|
d47cc43eab | ||
|
Gergo Magyar
|
348d0c9176 |
@@ -276,13 +276,22 @@ export async function retryRename(src: string, dst: string, attempts = 3): Promi
|
||||
|
||||
export async function writeBridgeMeta(groupDir: string, meta: BridgeMeta): Promise<void> {
|
||||
const target = path.join(groupDir, 'meta.json');
|
||||
const tmp = `${target}.tmp.${Date.now()}`;
|
||||
await fsp.writeFile(tmp, JSON.stringify(meta, null, 2), 'utf-8');
|
||||
// Use retryRename for consistency with writeBridge's atomic swap — on
|
||||
// Windows a concurrent reader can cause EBUSY/EPERM even on a tiny
|
||||
// meta.json, and we don't want meta write to be less robust than the
|
||||
// bridge.lbug swap it accompanies.
|
||||
await retryRename(tmp, target);
|
||||
// Stage inside a unique mkdtemp directory — the canonical CodeQL-
|
||||
// recognized sanitizer for js/insecure-temporary-file. The previous
|
||||
// shape (`${target}.tmp.${randomBytes()}` + `flag: 'wx'`) was
|
||||
// semantically equivalent but not on the analyzer's recognized list
|
||||
// and re-fired the alert. mkdtemp is anchored inside groupDir so the
|
||||
// rename stays on the same filesystem (no EXDEV).
|
||||
const stagingDir = await fsp.mkdtemp(path.join(groupDir, 'meta-tmp-'));
|
||||
try {
|
||||
const stagingPath = path.join(stagingDir, 'meta.json');
|
||||
await fsp.writeFile(stagingPath, JSON.stringify(meta, null, 2), 'utf-8');
|
||||
// retryRename handles transient EBUSY/EPERM on Windows for the same
|
||||
// reason writeBridge does — concurrent readers can lock meta.json.
|
||||
await retryRename(stagingPath, target);
|
||||
} finally {
|
||||
await fsp.rm(stagingDir, { recursive: true, force: true });
|
||||
}
|
||||
}
|
||||
|
||||
export async function readBridgeMeta(groupDir: string): Promise<BridgeMeta> {
|
||||
@@ -346,7 +355,19 @@ export async function writeBridge(
|
||||
const crossLinks = dedupeCrossLinks(input.crossLinks);
|
||||
|
||||
const finalPath = path.join(groupDir, 'bridge.lbug');
|
||||
const tmpPath = path.join(groupDir, 'bridge.lbug.tmp');
|
||||
// Stage the temp database inside a unique mkdtemp directory rather than
|
||||
// a fixed `bridge.lbug.tmp` name. The previous shape was flagged by
|
||||
// CodeQL js/insecure-temporary-file as a predictable path: a co-located
|
||||
// attacker (or a parallel writeBridge call into the same group) could
|
||||
// pre-create or symlink that path before this writer opens it. mkdtemp
|
||||
// returns a directory whose suffix is filled with cryptographically
|
||||
// random bytes, so the staging path is unguessable AND collision-free
|
||||
// across parallel callers. We anchor the staging directory inside
|
||||
// `groupDir` so the subsequent rename of `bridge.lbug` (and its
|
||||
// `.wal` / `.shadow` sidecars) into place stays on the same filesystem
|
||||
// and remains atomic — moving across `os.tmpdir()` could trip EXDEV.
|
||||
const stagingDir = await fsp.mkdtemp(path.join(groupDir, 'bridge-tmp-'));
|
||||
const tmpPath = path.join(stagingDir, 'bridge.lbug');
|
||||
const bakPath = path.join(groupDir, 'bridge.lbug.bak');
|
||||
|
||||
const report: WriteBridgeReport = {
|
||||
@@ -366,42 +387,42 @@ export async function writeBridge(
|
||||
}
|
||||
};
|
||||
|
||||
// Clean up any leftover tmp main file AND its `.wal` / `.shadow` sidecars.
|
||||
// LadybugDB 0.16.0 rejects opening a database whose sidecars belong to a
|
||||
// different database instance (database-id check), so any stale sidecar
|
||||
// from a crashed previous run will fail the next writeBridge.
|
||||
await removeLbugFile(tmpPath);
|
||||
// The mkdtemp staging directory above is freshly created with a unique
|
||||
// random suffix, so there are no leftover `bridge.lbug.tmp` / `.wal` /
|
||||
// `.shadow` sidecars from a previous crashed run to clean up here — the
|
||||
// directory is empty by construction.
|
||||
|
||||
// 1. Create temp DB, insert all data.
|
||||
//
|
||||
// Everything after `openBridgeDb` must run inside a try/finally so that
|
||||
// if ANY step before the explicit `closeBridgeDb` throws — schema
|
||||
// creation, a contract insert loop that rethrows, a snapshot write, the
|
||||
// cross-link loop, or anything else — the handle is still released. A
|
||||
// leaked handle holds the native LadybugDB file lock on tmpPath, which
|
||||
// (a) leaks a FD and (b) prevents the next writeBridge call from
|
||||
// reusing the same tmp slot.
|
||||
const handle = await openBridgeDb(tmpPath);
|
||||
let handleClosed = false;
|
||||
try {
|
||||
await ensureBridgeSchema(handle);
|
||||
// 1. Create temp DB, insert all data.
|
||||
//
|
||||
// Everything after `openBridgeDb` must run inside a try/finally so that
|
||||
// if ANY step before the explicit `closeBridgeDb` throws — schema
|
||||
// creation, a contract insert loop that rethrows, a snapshot write, the
|
||||
// cross-link loop, or anything else — the handle is still released. A
|
||||
// leaked handle holds the native LadybugDB file lock on tmpPath, which
|
||||
// (a) leaks a FD and (b) prevents the next writeBridge call from
|
||||
// reusing the same tmp slot.
|
||||
const handle = await openBridgeDb(tmpPath);
|
||||
let handleClosed = false;
|
||||
try {
|
||||
await ensureBridgeSchema(handle);
|
||||
|
||||
// Build the lookup index incrementally as contracts are inserted, so
|
||||
// failed inserts are never in the index (and therefore never resolved
|
||||
// by the cross-link loop below). This replaces a previous N+1 query
|
||||
// pattern where each link made up to 6 DB round-trips to find its
|
||||
// endpoints — see ContractLookupIndex.
|
||||
const lookupIndex = createContractLookupIndex();
|
||||
// Build the lookup index incrementally as contracts are inserted, so
|
||||
// failed inserts are never in the index (and therefore never resolved
|
||||
// by the cross-link loop below). This replaces a previous N+1 query
|
||||
// pattern where each link made up to 6 DB round-trips to find its
|
||||
// endpoints — see ContractLookupIndex.
|
||||
const lookupIndex = createContractLookupIndex();
|
||||
|
||||
// Insert contracts — tolerate individual failures (e.g., a corrupt meta
|
||||
// that can't be serialized). The whole sync must not fail because one
|
||||
// contract is broken.
|
||||
for (const c of contracts) {
|
||||
const id = contractNodeId(c.repo, c.contractId, c.role, c.symbolRef.filePath);
|
||||
try {
|
||||
await queryBridge(
|
||||
handle,
|
||||
`CREATE (n:Contract {
|
||||
// Insert contracts — tolerate individual failures (e.g., a corrupt meta
|
||||
// that can't be serialized). The whole sync must not fail because one
|
||||
// contract is broken.
|
||||
for (const c of contracts) {
|
||||
const id = contractNodeId(c.repo, c.contractId, c.role, c.symbolRef.filePath);
|
||||
try {
|
||||
await queryBridge(
|
||||
handle,
|
||||
`CREATE (n:Contract {
|
||||
id: $id,
|
||||
contractId: $contractId,
|
||||
type: $type,
|
||||
@@ -414,91 +435,91 @@ export async function writeBridge(
|
||||
confidence: $confidence,
|
||||
meta: $meta
|
||||
})`,
|
||||
{
|
||||
id,
|
||||
contractId: c.contractId,
|
||||
type: c.type,
|
||||
role: c.role,
|
||||
repo: c.repo,
|
||||
service: c.service ?? '',
|
||||
symbolUid: c.symbolUid,
|
||||
filePath: c.symbolRef.filePath,
|
||||
symbolName: c.symbolName,
|
||||
confidence: c.confidence,
|
||||
meta: JSON.stringify(c.meta),
|
||||
},
|
||||
);
|
||||
report.contractsInserted++;
|
||||
// Only index on successful insert — the cross-link loop must never
|
||||
// resolve to a row that isn't actually in the DB.
|
||||
indexContract(lookupIndex, c, id);
|
||||
} catch (err) {
|
||||
report.contractsFailed++;
|
||||
recordError('contract', id, err);
|
||||
{
|
||||
id,
|
||||
contractId: c.contractId,
|
||||
type: c.type,
|
||||
role: c.role,
|
||||
repo: c.repo,
|
||||
service: c.service ?? '',
|
||||
symbolUid: c.symbolUid,
|
||||
filePath: c.symbolRef.filePath,
|
||||
symbolName: c.symbolName,
|
||||
confidence: c.confidence,
|
||||
meta: JSON.stringify(c.meta),
|
||||
},
|
||||
);
|
||||
report.contractsInserted++;
|
||||
// Only index on successful insert — the cross-link loop must never
|
||||
// resolve to a row that isn't actually in the DB.
|
||||
indexContract(lookupIndex, c, id);
|
||||
} catch (err) {
|
||||
report.contractsFailed++;
|
||||
recordError('contract', id, err);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Insert repo snapshots
|
||||
for (const [repoId, snap] of Object.entries(input.repoSnapshots)) {
|
||||
try {
|
||||
await queryBridge(
|
||||
handle,
|
||||
`CREATE (s:RepoSnapshot {
|
||||
// Insert repo snapshots
|
||||
for (const [repoId, snap] of Object.entries(input.repoSnapshots)) {
|
||||
try {
|
||||
await queryBridge(
|
||||
handle,
|
||||
`CREATE (s:RepoSnapshot {
|
||||
id: $id,
|
||||
indexedAt: $indexedAt,
|
||||
lastCommit: $lastCommit
|
||||
})`,
|
||||
{
|
||||
id: repoId,
|
||||
indexedAt: snap.indexedAt,
|
||||
lastCommit: snap.lastCommit,
|
||||
},
|
||||
);
|
||||
report.snapshotsInserted++;
|
||||
} catch (err) {
|
||||
report.snapshotsFailed++;
|
||||
recordError('snapshot', repoId, err);
|
||||
}
|
||||
}
|
||||
|
||||
// Insert cross-links (tolerating missing nodes).
|
||||
//
|
||||
// `findContractNode` consults the in-memory lookup index built above,
|
||||
// not the DB — that's an O(1) pure-function lookup per endpoint instead
|
||||
// of the previous 2-3 DB queries. For M cross-links, the previous code
|
||||
// issued up to 6M round-trips; this version issues zero.
|
||||
//
|
||||
// `link.contractId` may differ between the consumer and provider sides
|
||||
// (e.g. wildcard consumer `grpc::Service/*` → method-level provider
|
||||
// `grpc::Service/Method`) — that's why we resolve each endpoint
|
||||
// independently via its own `(repo, role, symbolUid, filePath, symbolName)`
|
||||
// tuple rather than matching on contractId.
|
||||
for (const link of crossLinks) {
|
||||
const linkId = `${link.from.repo}::${link.contractId}->${link.to.repo}::${link.contractId}`;
|
||||
try {
|
||||
const fromId = findContractNode(
|
||||
lookupIndex,
|
||||
link.from.repo,
|
||||
'consumer',
|
||||
link.from.symbolUid,
|
||||
link.from.symbolRef.filePath,
|
||||
link.from.symbolRef.name,
|
||||
);
|
||||
const toId = findContractNode(
|
||||
lookupIndex,
|
||||
link.to.repo,
|
||||
'provider',
|
||||
link.to.symbolUid,
|
||||
link.to.symbolRef.filePath,
|
||||
link.to.symbolRef.name,
|
||||
);
|
||||
if (!fromId || !toId) {
|
||||
report.linksDroppedMissingNode++;
|
||||
continue;
|
||||
{
|
||||
id: repoId,
|
||||
indexedAt: snap.indexedAt,
|
||||
lastCommit: snap.lastCommit,
|
||||
},
|
||||
);
|
||||
report.snapshotsInserted++;
|
||||
} catch (err) {
|
||||
report.snapshotsFailed++;
|
||||
recordError('snapshot', repoId, err);
|
||||
}
|
||||
await queryBridge(
|
||||
handle,
|
||||
`
|
||||
}
|
||||
|
||||
// Insert cross-links (tolerating missing nodes).
|
||||
//
|
||||
// `findContractNode` consults the in-memory lookup index built above,
|
||||
// not the DB — that's an O(1) pure-function lookup per endpoint instead
|
||||
// of the previous 2-3 DB queries. For M cross-links, the previous code
|
||||
// issued up to 6M round-trips; this version issues zero.
|
||||
//
|
||||
// `link.contractId` may differ between the consumer and provider sides
|
||||
// (e.g. wildcard consumer `grpc::Service/*` → method-level provider
|
||||
// `grpc::Service/Method`) — that's why we resolve each endpoint
|
||||
// independently via its own `(repo, role, symbolUid, filePath, symbolName)`
|
||||
// tuple rather than matching on contractId.
|
||||
for (const link of crossLinks) {
|
||||
const linkId = `${link.from.repo}::${link.contractId}->${link.to.repo}::${link.contractId}`;
|
||||
try {
|
||||
const fromId = findContractNode(
|
||||
lookupIndex,
|
||||
link.from.repo,
|
||||
'consumer',
|
||||
link.from.symbolUid,
|
||||
link.from.symbolRef.filePath,
|
||||
link.from.symbolRef.name,
|
||||
);
|
||||
const toId = findContractNode(
|
||||
lookupIndex,
|
||||
link.to.repo,
|
||||
'provider',
|
||||
link.to.symbolUid,
|
||||
link.to.symbolRef.filePath,
|
||||
link.to.symbolRef.name,
|
||||
);
|
||||
if (!fromId || !toId) {
|
||||
report.linksDroppedMissingNode++;
|
||||
continue;
|
||||
}
|
||||
await queryBridge(
|
||||
handle,
|
||||
`
|
||||
MATCH (a:Contract), (b:Contract)
|
||||
WHERE a.id = $fromId AND b.id = $toId
|
||||
CREATE (a)-[:ContractLink {
|
||||
@@ -509,83 +530,93 @@ export async function writeBridge(
|
||||
toRepo: $toRepo
|
||||
}]->(b)
|
||||
`,
|
||||
{
|
||||
fromId,
|
||||
toId,
|
||||
matchType: link.matchType,
|
||||
confidence: link.confidence,
|
||||
contractId: link.contractId,
|
||||
fromRepo: link.from.repo,
|
||||
toRepo: link.to.repo,
|
||||
},
|
||||
);
|
||||
report.linksInserted++;
|
||||
} catch (err) {
|
||||
report.linksFailed++;
|
||||
recordError('link', linkId, err);
|
||||
{
|
||||
fromId,
|
||||
toId,
|
||||
matchType: link.matchType,
|
||||
confidence: link.confidence,
|
||||
contractId: link.contractId,
|
||||
fromRepo: link.from.repo,
|
||||
toRepo: link.to.repo,
|
||||
},
|
||||
);
|
||||
report.linksInserted++;
|
||||
} catch (err) {
|
||||
report.linksFailed++;
|
||||
recordError('link', linkId, err);
|
||||
}
|
||||
}
|
||||
|
||||
// 2. Close temp DB (happy path). The finally block also calls
|
||||
// closeBridgeDb if we threw above; `handleClosed` prevents a
|
||||
// double-close on the native handle.
|
||||
await closeBridgeDb(handle);
|
||||
handleClosed = true;
|
||||
} finally {
|
||||
if (!handleClosed) {
|
||||
await closeBridgeDb(handle).catch(() => {
|
||||
/* ignore: cleanup path, best effort */
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
// 2. Close temp DB (happy path). The finally block also calls
|
||||
// closeBridgeDb if we threw above; `handleClosed` prevents a
|
||||
// double-close on the native handle.
|
||||
await closeBridgeDb(handle);
|
||||
handleClosed = true;
|
||||
} finally {
|
||||
if (!handleClosed) {
|
||||
await closeBridgeDb(handle).catch(() => {
|
||||
/* ignore: cleanup path, best effort */
|
||||
});
|
||||
// 3. Atomic swap: old→.bak, tmp→final, rm .bak
|
||||
//
|
||||
// The current database file (with its `.wal` / `.shadow` sidecars) is
|
||||
// moved aside, then the freshly built tmp database takes its place.
|
||||
// We move the sidecars together with the main file so the open below
|
||||
// and any external readers see a consistent set; orphan sidecars from
|
||||
// the tmp namespace are then removed because LadybugDB looks for them
|
||||
// under the renamed-to base name and would reject mismatching IDs.
|
||||
try {
|
||||
await fsp.access(finalPath);
|
||||
await retryRename(finalPath, bakPath);
|
||||
for (const suffix of LBUG_SIDECAR_SUFFIXES) {
|
||||
try {
|
||||
await fsp.access(`${finalPath}${suffix}`);
|
||||
await retryRename(`${finalPath}${suffix}`, `${bakPath}${suffix}`);
|
||||
} catch {
|
||||
/* sidecar absent — nothing to move */
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
/* no existing db */
|
||||
}
|
||||
}
|
||||
|
||||
// 3. Atomic swap: old→.bak, tmp→final, rm .bak
|
||||
//
|
||||
// The current database file (with its `.wal` / `.shadow` sidecars) is
|
||||
// moved aside, then the freshly built tmp database takes its place.
|
||||
// We move the sidecars together with the main file so the open below
|
||||
// and any external readers see a consistent set; orphan sidecars from
|
||||
// the tmp namespace are then removed because LadybugDB looks for them
|
||||
// under the renamed-to base name and would reject mismatching IDs.
|
||||
try {
|
||||
await fsp.access(finalPath);
|
||||
await retryRename(finalPath, bakPath);
|
||||
await retryRename(tmpPath, finalPath);
|
||||
for (const suffix of LBUG_SIDECAR_SUFFIXES) {
|
||||
// Rename — not delete — so the WAL (which may carry uncommitted-at-
|
||||
// close-time pages on a graceful close, depending on
|
||||
// `autoCheckpoint` / `checkpointThreshold`) and the `.shadow`
|
||||
// checkpoint snapshot stay paired with the database file under its
|
||||
// final name. LadybugDB 0.16.0's database-id check rejects an open
|
||||
// when the sidecars belong to a different base name.
|
||||
try {
|
||||
await fsp.access(`${finalPath}${suffix}`);
|
||||
await retryRename(`${finalPath}${suffix}`, `${bakPath}${suffix}`);
|
||||
await fsp.access(`${tmpPath}${suffix}`);
|
||||
await retryRename(`${tmpPath}${suffix}`, `${finalPath}${suffix}`);
|
||||
} catch {
|
||||
/* sidecar absent — nothing to move */
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
/* no existing db */
|
||||
}
|
||||
await retryRename(tmpPath, finalPath);
|
||||
for (const suffix of LBUG_SIDECAR_SUFFIXES) {
|
||||
// Rename — not delete — so the WAL (which may carry uncommitted-at-
|
||||
// close-time pages on a graceful close, depending on
|
||||
// `autoCheckpoint` / `checkpointThreshold`) and the `.shadow`
|
||||
// checkpoint snapshot stay paired with the database file under its
|
||||
// final name. LadybugDB 0.16.0's database-id check rejects an open
|
||||
// when the sidecars belong to a different base name.
|
||||
try {
|
||||
await fsp.access(`${tmpPath}${suffix}`);
|
||||
await retryRename(`${tmpPath}${suffix}`, `${finalPath}${suffix}`);
|
||||
} catch {
|
||||
/* sidecar absent — nothing to move */
|
||||
}
|
||||
}
|
||||
await removeLbugFile(bakPath);
|
||||
await removeLbugFile(bakPath);
|
||||
|
||||
// 4. Write meta.json
|
||||
await writeBridgeMeta(groupDir, {
|
||||
version: BRIDGE_SCHEMA_VERSION,
|
||||
generatedAt: new Date().toISOString(),
|
||||
missingRepos: input.missingRepos,
|
||||
});
|
||||
// 4. Write meta.json
|
||||
await writeBridgeMeta(groupDir, {
|
||||
version: BRIDGE_SCHEMA_VERSION,
|
||||
generatedAt: new Date().toISOString(),
|
||||
missingRepos: input.missingRepos,
|
||||
});
|
||||
|
||||
return report;
|
||||
return report;
|
||||
} finally {
|
||||
// Always remove the mkdtemp staging directory. On the happy path the
|
||||
// main file and sidecars have been renamed out of it, so it's empty;
|
||||
// on any error path it may still contain a partial database — either
|
||||
// way `recursive: true, force: true` removes it without surfacing
|
||||
// "directory not empty" or ENOENT.
|
||||
await fsp.rm(stagingDir, { recursive: true, force: true }).catch(() => {
|
||||
/* best-effort cleanup */
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
/* ------------------------------------------------------------------ */
|
||||
@@ -682,11 +713,21 @@ export async function openBridgeDbReadOnly(groupDir: string): Promise<BridgeHand
|
||||
}
|
||||
}
|
||||
if (process.env.GITNEXUS_DEBUG_BRIDGE) {
|
||||
// Sanitize log values via JSON.stringify — the CodeQL-recognized
|
||||
// sanitizer for js/log-injection (CWE-117). The previous shape
|
||||
// (`.replace(/[\r\n]/g, ' ')`) closed the obvious CR/LF vector but
|
||||
// wasn't on the analyzer's recognized-sanitizer list and re-fired
|
||||
// the alert. JSON.stringify escapes CR, LF, quotes, backslashes, AND
|
||||
// C0 control characters (including ANSI escape sequences) to their
|
||||
// \\uXXXX form. The .slice(1, -1) removes the wrapping quotes for
|
||||
// readable inline log context.
|
||||
const safeMsg = JSON.stringify(
|
||||
lastErr instanceof Error ? lastErr.message : String(lastErr),
|
||||
).slice(1, -1);
|
||||
const safeDir = JSON.stringify(String(groupDir)).slice(1, -1);
|
||||
console.warn(
|
||||
`[bridge-db] openBridgeDbReadOnly(${groupDir}) gave up after ` +
|
||||
`${LBUG_OPEN_RETRY_ATTEMPTS} attempts: ${
|
||||
lastErr instanceof Error ? lastErr.message : String(lastErr)
|
||||
}`,
|
||||
`[bridge-db] openBridgeDbReadOnly(${safeDir}) gave up after ` +
|
||||
`${LBUG_OPEN_RETRY_ATTEMPTS} attempts: ${safeMsg}`,
|
||||
);
|
||||
}
|
||||
return null;
|
||||
|
||||
@@ -3,6 +3,7 @@ import * as fsp from 'node:fs/promises';
|
||||
import * as path from 'node:path';
|
||||
import * as os from 'node:os';
|
||||
import type { ContractRegistry } from './types.js';
|
||||
import { retryRename } from './bridge-db.js';
|
||||
|
||||
const CONTRACTS_FILE = 'contracts.json';
|
||||
|
||||
@@ -34,10 +35,26 @@ export async function writeContractRegistry(
|
||||
registry: ContractRegistry,
|
||||
): Promise<void> {
|
||||
const targetPath = path.join(groupDir, CONTRACTS_FILE);
|
||||
const tmpPath = `${targetPath}.tmp.${Date.now()}`;
|
||||
|
||||
await fsp.writeFile(tmpPath, JSON.stringify(registry, null, 2), 'utf-8');
|
||||
await fsp.rename(tmpPath, targetPath);
|
||||
// Stage inside a unique mkdtemp directory rather than writing a tmp file
|
||||
// alongside the target. CodeQL's js/insecure-temporary-file query
|
||||
// recognizes mkdtemp-staging as a sanitizer (see writeBridge in
|
||||
// bridge-db.ts and the U6 follow-up plan). The previous shape
|
||||
// (`${target}.tmp.${randomBytes()}` + `flag: 'wx'`) was semantically
|
||||
// equivalent but not on CodeQL's recognized-sanitizer list; alerts
|
||||
// re-fired on the new code. mkdtemp gives us collision-free, unguessable
|
||||
// staging anchored inside groupDir so the rename stays on the same
|
||||
// filesystem (no EXDEV) and is atomic.
|
||||
const stagingDir = await fsp.mkdtemp(path.join(groupDir, 'contracts-tmp-'));
|
||||
try {
|
||||
const stagingPath = path.join(stagingDir, CONTRACTS_FILE);
|
||||
await fsp.writeFile(stagingPath, JSON.stringify(registry, null, 2), 'utf-8');
|
||||
await retryRename(stagingPath, targetPath);
|
||||
} finally {
|
||||
// Best-effort cleanup. On the happy path the file was renamed out, so
|
||||
// the staging dir is empty. On a write/rename failure it may contain a
|
||||
// partial file; we remove it either way to avoid disk leak.
|
||||
await fsp.rm(stagingDir, { recursive: true, force: true });
|
||||
}
|
||||
}
|
||||
|
||||
export async function readContractRegistry(groupDir: string): Promise<ContractRegistry | null> {
|
||||
@@ -77,10 +94,21 @@ export async function createGroupDir(
|
||||
force: boolean = false,
|
||||
): Promise<string> {
|
||||
const groupDir = getGroupDir(gitnexusDir, groupName);
|
||||
|
||||
// The existsSync check is UX only — provides a friendly "already exists"
|
||||
// error so users don't see a raw EEXIST. The real security guard is the
|
||||
// mkdtemp-staging + atomic-directory-rename pattern below: even if a
|
||||
// concurrent caller creates the group between the existsSync return and
|
||||
// the rename, the rename will fail (rather than silently overwrite a
|
||||
// half-built group). CodeQL js/insecure-temporary-file recognizes the
|
||||
// mkdtemp idiom as a sanitizer; the previous `flag: 'wx'` shape was
|
||||
// semantically equivalent but not on the recognized list.
|
||||
if (fs.existsSync(path.join(groupDir, 'group.yaml')) && !force) {
|
||||
throw new Error(`Group "${groupName}" already exists. Use --force to overwrite.`);
|
||||
}
|
||||
await fsp.mkdir(groupDir, { recursive: true });
|
||||
|
||||
const groupsBaseDir = path.dirname(groupDir);
|
||||
await fsp.mkdir(groupsBaseDir, { recursive: true });
|
||||
|
||||
const template = `version: 1
|
||||
name: ${groupName}
|
||||
@@ -106,6 +134,29 @@ matching:
|
||||
# exclude_links_paths: [/ping, /health, /healthcheck]
|
||||
# exclude_links_param_only_paths: false
|
||||
`;
|
||||
await fsp.writeFile(path.join(groupDir, 'group.yaml'), template, 'utf-8');
|
||||
|
||||
// Stage the entire group directory in a sibling mkdtemp directory, then
|
||||
// rename it into place atomically. On POSIX, rename(2) of a directory is
|
||||
// atomic when target doesn't exist, and atomic-replace when target does
|
||||
// (used here for force=true after rm). On Windows, the same pattern works
|
||||
// for non-existent targets; the force=true path explicitly removes the
|
||||
// existing groupDir first.
|
||||
const stagingDir = await fsp.mkdtemp(path.join(groupsBaseDir, `init-${groupName}-`));
|
||||
let renamed = false;
|
||||
try {
|
||||
await fsp.writeFile(path.join(stagingDir, 'group.yaml'), template, 'utf-8');
|
||||
if (force) {
|
||||
await fsp.rm(groupDir, { recursive: true, force: true });
|
||||
}
|
||||
await retryRename(stagingDir, groupDir);
|
||||
renamed = true;
|
||||
} finally {
|
||||
// Only clean up the staging dir if the rename didn't consume it. After
|
||||
// a successful rename, stagingDir is now groupDir — removing it would
|
||||
// delete the group we just created.
|
||||
if (!renamed) {
|
||||
await fsp.rm(stagingDir, { recursive: true, force: true });
|
||||
}
|
||||
}
|
||||
return groupDir;
|
||||
}
|
||||
|
||||
@@ -0,0 +1,160 @@
|
||||
/**
|
||||
* Regression tests for U6 — closes CodeQL js/insecure-temporary-file
|
||||
* (#191/#192/#193 originally; #467/#468/#469 after first-pass refactor)
|
||||
* and js/log-injection (#188 originally; #466 after first-pass refactor)
|
||||
* in core/group.
|
||||
*
|
||||
* The current shape uses fs.mkdtemp staging directories + retryRename for
|
||||
* atomic-write semantics — the canonical CodeQL-recognized sanitizer for
|
||||
* js/insecure-temporary-file. Tests pin every fix path so a future refactor
|
||||
* that drops mkdtemp, drops the finally-cleanup, or re-introduces the
|
||||
* predictable-suffix pattern would fail at least one test AND re-trigger
|
||||
* the corresponding CodeQL alert.
|
||||
*/
|
||||
import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest';
|
||||
import path from 'node:path';
|
||||
import fs from 'node:fs/promises';
|
||||
import os from 'node:os';
|
||||
import { writeContractRegistry, createGroupDir } from '../../../src/core/group/storage.js';
|
||||
import { writeBridgeMeta } from '../../../src/core/group/bridge-db.js';
|
||||
|
||||
let tmpRoot: string;
|
||||
let groupDir: string;
|
||||
|
||||
beforeAll(async () => {
|
||||
tmpRoot = await fs.mkdtemp(path.join(os.tmpdir(), 'gitnexus-u6-'));
|
||||
groupDir = path.join(tmpRoot, 'fixture-group');
|
||||
await fs.mkdir(groupDir, { recursive: true });
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await fs.rm(tmpRoot, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
// Helper: enumerate the staging directories that a function may have left
|
||||
// behind. Used to assert the finally-cleanup invariant. The mkdtemp prefix
|
||||
// pattern matches the production code: writeContractRegistry uses
|
||||
// `contracts-tmp-`, writeBridgeMeta uses `meta-tmp-`, createGroupDir uses
|
||||
// `init-${groupName}-` inside the parent (groups) directory.
|
||||
async function listStagingDirs(parentDir: string, prefix: string): Promise<string[]> {
|
||||
const entries = await fs.readdir(parentDir, { withFileTypes: true });
|
||||
return entries.filter((e) => e.isDirectory() && e.name.startsWith(prefix)).map((e) => e.name);
|
||||
}
|
||||
|
||||
describe('writeContractRegistry — mkdtemp staging hardening', () => {
|
||||
it('writes the file then leaves no contracts-tmp- staging directory behind', async () => {
|
||||
const dir = await fs.mkdtemp(path.join(tmpRoot, 'wcr-cleanup-'));
|
||||
await writeContractRegistry(dir, { contracts: [], version: 1 } as never);
|
||||
expect(await listStagingDirs(dir, 'contracts-tmp-')).toEqual([]);
|
||||
const written = JSON.parse(await fs.readFile(path.join(dir, 'contracts.json'), 'utf-8'));
|
||||
expect(written.version).toBe(1);
|
||||
});
|
||||
|
||||
it('back-to-back writes succeed and the final file matches the second write', async () => {
|
||||
const dir = await fs.mkdtemp(path.join(tmpRoot, 'wcr-back2back-'));
|
||||
await writeContractRegistry(dir, { contracts: [], version: 1 } as never);
|
||||
await writeContractRegistry(dir, { contracts: [], version: 2 } as never);
|
||||
expect(await listStagingDirs(dir, 'contracts-tmp-')).toEqual([]);
|
||||
const written = JSON.parse(await fs.readFile(path.join(dir, 'contracts.json'), 'utf-8'));
|
||||
expect(written.version).toBe(2);
|
||||
});
|
||||
|
||||
it('cleans up the staging directory even when the write fails', async () => {
|
||||
// Force a write failure by passing a value that JSON.stringify rejects
|
||||
// (a circular-reference object). The mkdtemp call still succeeds; the
|
||||
// writeFile inside it throws. The finally block must remove the
|
||||
// staging dir anyway. This test would fail if a future refactor
|
||||
// dropped the finally cleanup.
|
||||
const dir = await fs.mkdtemp(path.join(tmpRoot, 'wcr-failure-'));
|
||||
const circular: Record<string, unknown> = {};
|
||||
circular.self = circular;
|
||||
await expect(
|
||||
writeContractRegistry(dir, { contracts: [], extra: circular } as never),
|
||||
).rejects.toThrow();
|
||||
expect(await listStagingDirs(dir, 'contracts-tmp-')).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('writeBridgeMeta — mkdtemp staging hardening', () => {
|
||||
it('writes meta.json then leaves no meta-tmp- staging directory behind', async () => {
|
||||
const dir = await fs.mkdtemp(path.join(tmpRoot, 'wbm-cleanup-'));
|
||||
await writeBridgeMeta(dir, { version: 1, generatedAt: 'a', missingRepos: [] });
|
||||
expect(await listStagingDirs(dir, 'meta-tmp-')).toEqual([]);
|
||||
const meta = JSON.parse(await fs.readFile(path.join(dir, 'meta.json'), 'utf-8'));
|
||||
expect(meta.version).toBe(1);
|
||||
});
|
||||
|
||||
it('back-to-back writes leave no staging dirs behind', async () => {
|
||||
const dir = await fs.mkdtemp(path.join(tmpRoot, 'wbm-back2back-'));
|
||||
await writeBridgeMeta(dir, { version: 1, generatedAt: 'a', missingRepos: [] });
|
||||
await writeBridgeMeta(dir, { version: 2, generatedAt: 'b', missingRepos: [] });
|
||||
expect(await listStagingDirs(dir, 'meta-tmp-')).toEqual([]);
|
||||
const meta = JSON.parse(await fs.readFile(path.join(dir, 'meta.json'), 'utf-8'));
|
||||
expect(meta.version).toBe(2);
|
||||
});
|
||||
});
|
||||
|
||||
describe('createGroupDir — atomic-directory mkdtemp + rename', () => {
|
||||
it('creates the group with group.yaml and leaves no init- staging dirs', async () => {
|
||||
const gnxDir = path.join(tmpRoot, 'cgd-clean');
|
||||
await createGroupDir(gnxDir, 'mygroup');
|
||||
const groupsDir = path.join(gnxDir, 'groups');
|
||||
expect(await listStagingDirs(groupsDir, 'init-mygroup-')).toEqual([]);
|
||||
const yaml = await fs.readFile(path.join(groupsDir, 'mygroup', 'group.yaml'), 'utf-8');
|
||||
expect(yaml).toContain('name: mygroup');
|
||||
});
|
||||
|
||||
it('refuses to overwrite an existing group without force', async () => {
|
||||
const gnxDir = path.join(tmpRoot, 'cgd-existing');
|
||||
await createGroupDir(gnxDir, 'mygroup');
|
||||
await expect(createGroupDir(gnxDir, 'mygroup')).rejects.toThrow(/already exists/);
|
||||
// Even on the rejected path, no leftover staging dir.
|
||||
expect(await listStagingDirs(path.join(gnxDir, 'groups'), 'init-mygroup-')).toEqual([]);
|
||||
});
|
||||
|
||||
it('overwrites with force=true and still cleans up staging', async () => {
|
||||
const gnxDir = path.join(tmpRoot, 'cgd-force');
|
||||
await createGroupDir(gnxDir, 'mygroup');
|
||||
await createGroupDir(gnxDir, 'mygroup', true);
|
||||
expect(await listStagingDirs(path.join(gnxDir, 'groups'), 'init-mygroup-')).toEqual([]);
|
||||
const yaml = await fs.readFile(path.join(gnxDir, 'groups', 'mygroup', 'group.yaml'), 'utf-8');
|
||||
expect(yaml).toContain('name: mygroup');
|
||||
});
|
||||
});
|
||||
|
||||
describe('openBridgeDbReadOnly debug-warn — JSON.stringify log sanitizer', () => {
|
||||
it('strips CR/LF from groupDir and error message via JSON.stringify', () => {
|
||||
// The fix uses JSON.stringify(value).slice(1, -1). This pinning verifies
|
||||
// the sanitizer expression directly against an injected payload — the
|
||||
// production code uses the identical idiom inline.
|
||||
const evil = '/tmp/group\r\n2026-01-01 [bridge-db] FAKE LINE injected';
|
||||
const sanitized = JSON.stringify(evil).slice(1, -1);
|
||||
expect(sanitized).not.toContain('\n');
|
||||
expect(sanitized).not.toContain('\r');
|
||||
// JSON.stringify escapes CR/LF to their \r and \n literal-backslash forms.
|
||||
expect(sanitized).toContain('\\r');
|
||||
expect(sanitized).toContain('\\n');
|
||||
// Single line — no actual newline reaches the log stream.
|
||||
expect(sanitized.split('\n').length).toBe(1);
|
||||
});
|
||||
|
||||
it('escapes ANSI/C0 control characters as a defense-in-depth side effect', () => {
|
||||
// CR/LF stripping was the CodeQL-flagged class. JSON.stringify is
|
||||
// strictly stronger — it also escapes ANSI escape sequences and other
|
||||
// C0 control characters that could manipulate terminal output. Pin
|
||||
// this so a future revert to a CR/LF-only sanitizer is visible.
|
||||
const ansi = '[31mRED[0m';
|
||||
const sanitized = JSON.stringify(ansi).slice(1, -1);
|
||||
expect(sanitized).not.toContain('');
|
||||
expect(sanitized).toMatch(/\\u001[bB]/);
|
||||
});
|
||||
|
||||
// Tracking note: a true integration test against the live console.warn
|
||||
// call would require triggering openBridgeDbReadOnly's retry-exhaustion
|
||||
// path with a mocked lbug throwing a CRLF-containing error. The retry
|
||||
// path uses LBUG_OPEN_RETRY_ATTEMPTS attempts (currently 3) with backoff
|
||||
// — too slow and brittle for the unit suite. The pure-function pinning
|
||||
// above is what verifies the sanitizer; vi.spyOn is unused but kept
|
||||
// available via the import in case integration coverage is added later.
|
||||
void vi; // suppress unused-import warning
|
||||
});
|
||||
Reference in New Issue
Block a user