Merge branch 'main' into feat/uq-publish

This commit is contained in:
Gergő Magyar 2026-05-08 07:44:44 +01:00 • committed by GitHub
commit d6ccc186f0
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 548 additions and 238 deletions

View file

@ -32,6 +32,7 @@ import pathlib
import re
import sys
import urllib.error
import urllib.parse
import urllib.request
REPO_ROOT = pathlib.Path(__file__).resolve().parents[2]
@ -190,7 +191,17 @@ def fetch_text(url: str, timeout: int = 8) -> str | None:
set (raises the rate limit from 60 to 5 000 requests/hour).
"""
headers: dict[str, str] = {}
if _GITHUB_TOKEN and ("github.com" in url or "githubusercontent.com" in url):
# Parse the URL and check the hostname rather than substring-matching
# on the full URL string (CodeQL py/incomplete-url-substring-sanitization).
# `https://evil.com/?u=github.com` would have passed the substring check.
try:
parsed_host = urllib.parse.urlparse(url).hostname or ""
except ValueError:
parsed_host = ""
is_github_host = parsed_host == "github.com" or parsed_host.endswith(
(".github.com", ".githubusercontent.com")
) or parsed_host == "githubusercontent.com"
if _GITHUB_TOKEN and is_github_host:
headers["Authorization"] = f"Bearer {_GITHUB_TOKEN}"
try:
req = urllib.request.Request(url, headers=headers)

View file

@ -277,8 +277,10 @@ const extractInstanceName = (endpoint: string): string => {
try {
const url = new URL(endpoint);
const hostname = url.hostname;
// Extract the first part before .openai.azure.com
const match = hostname.match(/^([^.]+)\.openai\.azure\.com/);
// Extract the first part before .openai.azure.com. The trailing `$`
// anchor is required (CodeQL js/regex/missing-regexp-anchor): without
// it `evil.openai.azure.com.attacker.tld` would match.
const match = hostname.match(/^([^.]+)\.openai\.azure\.com$/);
if (match) {
return match[1];
}

View file

@ -278,8 +278,11 @@ export const createGraphRAGTools = (backend: GraphRAGBackend) => {
const val = row[col];
if (val === null || val === undefined) return '';
if (typeof val === 'object') return JSON.stringify(val);
// Truncate long values and escape pipe characters
const str = String(val).replace(/\|/g, '\\|');
// Truncate long values and escape pipe characters. Escape
// backslashes FIRST so the subsequent pipe escape isn't
// unescaped by a trailing backslash (CodeQL
// js/incomplete-sanitization).
const str = String(val).replace(/\\/g, '\\\\').replace(/\|/g, '\\|');
return str.length > 60 ? str.slice(0, 57) + '...' : str;
});
return `| ${values.join(' | ')} |`;

View file

@ -365,7 +365,12 @@ async function installClaudeCodeHooks(result: SetupResult): Promise<void> {
}
const hookPath = path.join(destHooksDir, 'gitnexus-hook.cjs').replace(/\\/g, '/');
const hookCmd = `node "${hookPath.replace(/"/g, '\\"')}"`;
// Escape backslashes FIRST, then quotes (CodeQL js/incomplete-sanitization).
// The previous shape `replace(/"/g, '\\"')` alone would let `path\with"quote`
// become `path\with\"quote`, where the trailing `\` before `"` could
// unescape the quote inside the surrounding double-quoted shell context.
const escapedHookPath = hookPath.replace(/\\/g, '\\\\').replace(/"/g, '\\"');
const hookCmd = `node "${escapedHookPath}"`;
// Check which hook events need entries (idempotent: skip if already registered)
const parsed = await (async () => {

View file

@ -602,6 +602,38 @@ function hasGhCLI(): boolean {
}
}
/**
* Strict Gist URL predicate. Rejects:
* - any URL that does not parse (URL constructor throws)
* - schemes other than https (drops `http:`, `file:`, `gist:`-style spoofs)
* - hostnames that are not exactly `gist.github.com` (drops substring spoofs
* like `https://evil.com/?u=gist.github.com` and userinfo-prefixed shapes
* like `https://[email protected]/...` — note that URL.hostname
* strips userinfo, so the equality check rejects the userinfo-prefixed
* spoof if the actual host differs from gist.github.com)
* - any URL containing userinfo (`username[:password]@`), which the URL
* parser exposes via `.username` / `.password`. Defense-in-depth: even
* when hostname matches, a credential-bearing URL is suspect and not
* produced by `gh gist create`.
*
* Closes the substring-bypass class CodeQL `js/incomplete-url-substring-
* sanitization` flags.
*/
function isGistUrl(line: string): boolean {
const trimmed = line.trim();
try {
const u = new URL(trimmed);
return (
u.protocol === 'https:' &&
u.hostname === 'gist.github.com' &&
u.username === '' &&
u.password === ''
);
} catch {
return false;
}
}
function publishGist(htmlPath: string): { url: string; rawUrl: string } | null {
try {
const output = execFileSync(
@ -610,13 +642,14 @@ function publishGist(htmlPath: string): { url: string; rawUrl: string } | null {
{ encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'] },
).trim();
// gh gist create prints the gist URL as the last line
const lines = output.split('\n');
const gistUrl = lines.find((l) => l.includes('gist.github.com')) || lines[lines.length - 1];
// `gh gist create` prints the gist URL as a line in the output. Find the
// first parseable Gist URL — if no line is a valid Gist URL, fail closed
// (do NOT fall back to lines[last]: a non-Gist last line would propagate
// through the regex below and produce a malformed `rawUrl`).
const gistUrl = output.split('\n').find(isGistUrl);
if (!gistUrl) return null;
if (!gistUrl || !gistUrl.includes('gist.github.com')) return null;
// Build a raw viewer URL via gist.githack.com
// Build a raw viewer URL via gist.githack.com.
// gist URL format: https://gist.github.com/{user}/{id}
const match = gistUrl.match(/gist\.github\.com\/([^/]+)\/([a-f0-9]+)/);
let rawUrl = gistUrl;

View file

@ -44,26 +44,6 @@ async function removeLbugFile(basePath: string): Promise<void> {
}
}
/**
* Remove all stale `bridge.lbug.tmp.*` files (and their sidecars) from a
* group directory. With randomBytes-based temp names, a crashed writeBridge
* leaves behind a uniquely-named tmp file that no future run will target by
* name — so we glob for the prefix and clean up everything matching.
*/
async function cleanStaleBridgeTmpFiles(groupDir: string): Promise<void> {
try {
const entries = await fsp.readdir(groupDir);
const staleBases = entries.filter(
(e) => e.startsWith('bridge.lbug.tmp.') && !LBUG_SIDECAR_SUFFIXES.some((s) => e.endsWith(s)),
);
for (const name of staleBases) {
await removeLbugFile(path.join(groupDir, name));
}
} catch {
/* best-effort: directory may not exist yet */
}
}
export function contractNodeId(
repo: string,
contractId: string,
@ -299,8 +279,24 @@ 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');
// Unpredictable suffix + O_EXCL via `'wx'` flag closes the symlink/
// pre-create attack window. The third argument `0o600` is the
// user-only mode mask — CodeQL's `js/insecure-temporary-file` query
// sources its verdict from the `mode` argument, NOT from `flags`:
// its `isSecureMode(mode)` predicate requires the low 6 bits to be
// zero (no group/world bits). Without an explicit mode the file is
// created with the process umask (typically 0o644 = group/world
// readable), which the query treats as the actual vulnerability.
// Both `'wx'` (runtime O_EXCL) AND `0o600` (CodeQL-credited mode)
// are needed: one closes the symlink race, the other closes the
// permissions exposure.
const tmp = `${target}.tmp.${randomBytes(8).toString('hex')}`;
await fsp.writeFile(tmp, JSON.stringify(meta, null, 2), 'utf-8');
const handle = await fsp.open(tmp, 'wx', 0o600);
try {
await handle.writeFile(JSON.stringify(meta, null, 2), 'utf-8');
} finally {
await handle.close();
}
// 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
@ -369,7 +365,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.${randomBytes(8).toString('hex')}`);
// 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 = {
@ -389,43 +397,42 @@ export async function writeBridge(
}
};
// Clean up stale tmp files left behind by previously crashed writeBridge
// runs. With randomBytes-based names each run picks a unique path, so
// the old fixed-name `removeLbugFile(tmpPath)` was a no-op — stale
// artifacts accumulated. The glob-based helper finds *all* leftover
// `bridge.lbug.tmp.*` entries and removes them (including sidecars).
await cleanStaleBridgeTmpFiles(groupDir);
// 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,
@ -438,91 +445,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 {
@ -533,83 +540,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 */
});
}
}
/* ------------------------------------------------------------------ */
@ -705,6 +722,11 @@ export async function openBridgeDbReadOnly(groupDir: string): Promise<BridgeHand
await new Promise((r) => setTimeout(r, delay));
}
}
// Pino's NDJSON serialization is structurally injection-resistant
// (CodeQL js/log-injection): groupDir and err.message are JSON-escaped
// by the serializer, so no manual CRLF / U+2028 / ANSI sanitization is
// needed. Demoted to debug — only fires when the bridge truly gave up
// after retries, and operators only need it at debug verbosity.
bridgeLogger.debug(
{ groupDir, err: lastErr, attempts: LBUG_OPEN_RETRY_ATTEMPTS },
'openBridgeDbReadOnly gave up',

View file

@ -5,6 +5,15 @@ import * as os from 'node:os';
import { randomBytes } from 'node:crypto';
import type { ContractRegistry } from './types.js';
/**
* Build an unpredictable suffix for atomic-write tmp files. Replaces the
* previous `Date.now()` pattern which CodeQL flagged as
* js/insecure-temporary-file: a guessable suffix in a writable directory
* lets a co-located attacker pre-create or symlink the tmp path before the
* write lands.
*/
const tmpSuffix = (): string => randomBytes(8).toString('hex');
const CONTRACTS_FILE = 'contracts.json';
export function getDefaultGitnexusDir(): string {
@ -35,9 +44,21 @@ export async function writeContractRegistry(
registry: ContractRegistry,
): Promise<void> {
const targetPath = path.join(groupDir, CONTRACTS_FILE);
const tmpPath = `${targetPath}.tmp.${randomBytes(8).toString('hex')}`;
const tmpPath = `${targetPath}.tmp.${tmpSuffix()}`;
await fsp.writeFile(tmpPath, JSON.stringify(registry, null, 2), 'utf-8');
// O_EXCL via `'wx'` flag + explicit `0o600` mode — closes both halves
// of the CodeQL js/insecure-temporary-file finding: `'wx'` rejects a
// pre-planted symlink at the path, and `0o600` (user-only) prevents
// the file from being created group/world readable while it briefly
// contains contract data en route to the rename. The query's
// `isSecureMode` predicate inspects ONLY the mode argument, not the
// flags, so the explicit mode is what credits the fix.
const handle = await fsp.open(tmpPath, 'wx', 0o600);
try {
await handle.writeFile(JSON.stringify(registry, null, 2), 'utf-8');
} finally {
await handle.close();
}
await fsp.rename(tmpPath, targetPath);
}
@ -107,6 +128,38 @@ matching:
# exclude_links_paths: [/ping, /health, /healthcheck]
# exclude_links_param_only_paths: false
`;
await fsp.writeFile(path.join(groupDir, 'group.yaml'), template, 'utf-8');
// Always write group.yaml with O_EXCL via `fsp.open(..., 'wx')` —
// refuses to follow a pre-planted symlink at the target path, closing
// the TOCTOU window between the existence check (line ~98) and the
// write that CodeQL js/insecure-temporary-file flags. Under
// `force=true` we unlink the existing file first (best-effort, no-op
// when absent) so the subsequent O_EXCL open succeeds AND the same
// symlink-rejection guarantee holds — this is strictly safer than
// the previous `flag: force ? 'w' : 'wx'` shape, which silently
// followed symlinks under force. CodeQL's rule does not recognize
// the `writeFile(path, content, { flag: 'wx' })` shape as O_EXCL;
// the explicit open() handle below is what credits the mitigation.
const yamlPath = path.join(groupDir, 'group.yaml');
if (force) {
try {
await fsp.unlink(yamlPath);
} catch (err) {
// ENOENT (file absent) is expected on first run; rethrow anything
// else so we don't silently mask permission/EBUSY failures.
if ((err as NodeJS.ErrnoException).code !== 'ENOENT') throw err;
}
}
// `'wx'` rejects a pre-planted symlink at the path; `0o600` is
// user-only (no group/world bits) — gitnexus storage is per-user
// (`~/.gitnexus/...`), so any "other user wants to read this" case is
// a misconfiguration, not a feature. Keeping the file user-only also
// satisfies CodeQL's `isSecureMode` predicate (low 6 bits == 0) and
// closes the js/insecure-temporary-file alert at this site.
const handle = await fsp.open(yamlPath, 'wx', 0o600);
try {
await handle.writeFile(template, 'utf-8');
} finally {
await handle.close();
}
return groupDir;
}

View file

@ -23,7 +23,24 @@ interface ScriptBlock {
lang: string;
}
const SCRIPT_RE = /<script(\s[^>]*)?>([^]*?)<\/script>/g;
// Closing-tag pattern accepts:
// - whitespace before `>` — `</script >`, `</script\t\n>`
// - attribute-like junk after `script` — `</script foo="bar">`,
// `</script\t\n bar>`
// - any case — `</SCRIPT>`, `</Script>`
//
// HTML5 parses `</script foo>` as a valid close tag (attributes on
// close tags are ignored by the parser but still terminate the script
// block). A strict `<\/script\s*>` would miss those forms and let a
// crafted Vue file hide content from this extractor — exactly the
// CodeQL `js/bad-tag-filter` failure mode (the published test cases
// it checks include `</script foo="bar">` and `</script\t\n bar>`).
//
// `[^>]*` after `</script` accepts everything up to the next `>`,
// matching the HTML parser's actual close-tag behaviour. The `i` flag
// covers the case axis. PR #1330 CI surfaced both the case and
// attribute axes; this expression closes both at once.
const SCRIPT_RE = /<script(\s[^>]*)?>([^]*?)<\/script[^>]*>/gi;
const TEMPLATE_COMPONENT_RE = /<([A-Z][A-Za-z0-9]+)/g;
// Greedy: matches from the first <template> to the *last* </template>.
// This is intentional — nested <template v-slot:...> tags are valid Vue

View file

@ -86,8 +86,10 @@ export function isAzureProvider(baseUrl: string): boolean {
const { hostname } = new URL(baseUrl);
return hostname.endsWith('.openai.azure.com') || hostname.endsWith('.services.ai.azure.com');
} catch {
// If URL is malformed, fall back to substring check
return baseUrl.includes('.openai.azure.com') || baseUrl.includes('.services.ai.azure.com');
// Malformed URL — refuse to call this Azure rather than fall back to a
// substring check, which is bypassable by `https://evil.com/?u=.openai.azure.com`
// (CodeQL js/incomplete-url-substring-sanitization).
return false;
}
}

View file

@ -0,0 +1,90 @@
/**
* Regression tests for U6 — closes CodeQL js/insecure-temporary-file
* (#191/#192/#193) and js/log-injection (#188) in core/group.
*
* The fixes replace `Date.now()` suffix tmp files with crypto.randomBytes
* suffixes + open the tmp file with `flag: 'wx'` (O_EXCL). These tests
* pin both behaviors so a future refactor that drops either signal
* regenerates the CodeQL alert AND fails a test.
*/
import { afterAll, beforeAll, describe, expect, it } 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';
import type { ContractRegistry } from '../../../src/core/group/types.js';
/**
* Build a minimal `ContractRegistry` literal with overridable fields.
* Replaces the `as never` cast that bypassed the type entirely — keeps
* the test free of unrelated boilerplate while still type-checking the
* fields under test.
*/
function makeRegistry(overrides: Partial<ContractRegistry> = {}): ContractRegistry {
return {
version: 1,
generatedAt: '2026-05-07T00:00:00Z',
repoSnapshots: {},
missingRepos: [],
contracts: [],
crossLinks: [],
...overrides,
};
}
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 });
});
describe('writeContractRegistry — tempfile hardening', () => {
it('back-to-back writes within the same ms do not collide on the tmp path', async () => {
// The previous `${path}.tmp.${Date.now()}` shape collided when two writers
// landed in the same millisecond. crypto.randomBytes makes the suffix
// essentially-unique. Sequential writes here pin the unique-suffix
// property without depending on Windows-specific concurrent-rename
// behavior (which has its own pre-existing retry pattern in the
// sibling `writeBridge` function and is out of scope for this test).
await writeContractRegistry(groupDir, makeRegistry({ version: 1 }));
await writeContractRegistry(groupDir, makeRegistry({ version: 2 }));
const written = await fs.readFile(path.join(groupDir, 'contracts.json'), 'utf-8');
const parsed = JSON.parse(written);
expect(parsed.version).toBe(2);
});
});
describe('writeBridgeMeta — tempfile hardening', () => {
it('back-to-back writes do not collide on the tmp path', async () => {
await writeBridgeMeta(groupDir, { version: 1, generatedAt: 'a', missingRepos: [] });
await writeBridgeMeta(groupDir, { version: 2, generatedAt: 'b', missingRepos: [] });
const meta = JSON.parse(await fs.readFile(path.join(groupDir, 'meta.json'), 'utf-8'));
expect(meta.version).toBe(2);
});
});
describe('createGroupDir — exclusive-create on group.yaml', () => {
it('refuses to overwrite an existing group without force', async () => {
const gnxDir = path.join(tmpRoot, 'gnx-existing');
await createGroupDir(gnxDir, 'mygroup');
// Second call without force should throw — same behavior as before this
// commit, but now backed by O_EXCL at the writeFile level rather than
// only the up-front existence check (closes the TOCTOU CodeQL flagged).
await expect(createGroupDir(gnxDir, 'mygroup')).rejects.toThrow(/already exists/);
});
it('overwrites with force=true', async () => {
const gnxDir = path.join(tmpRoot, 'gnx-force');
await createGroupDir(gnxDir, 'mygroup');
// Should succeed without throwing.
await expect(createGroupDir(gnxDir, 'mygroup', true)).resolves.toBeTruthy();
});
});

View file

@ -43,45 +43,65 @@ describe('insecure tempfile — structural guards (#1318 U6)', () => {
expect(bridgeSource).toMatch(/import\s*\{[^}]*randomBytes[^}]*\}\s*from\s*'node:crypto'/);
});
it('bridge-db.ts uses randomBytes for bridge.lbug temp path', () => {
expect(bridgeSource).toMatch(/bridge\.lbug\.tmp\.\$\{randomBytes/);
it('bridge-db.ts uses mkdtemp staging directory for bridge.lbug', () => {
// Follow-up to the original randomBytes pattern: stage inside a
// mkdtemp-created directory so the suffix is OS-supplied, the
// directory contents are empty by construction, and concurrent
// writers cannot collide. The bridge.lbug filename inside that
// directory is fixed; uniqueness comes from the directory name.
expect(bridgeSource).toMatch(/fsp\.mkdtemp\(path\.join\(groupDir,\s*['"]bridge-tmp-['"]\)\)/);
expect(bridgeSource).toMatch(/path\.join\(stagingDir,\s*['"]bridge\.lbug['"]\)/);
});
it('bridge-db.ts uses randomBytes for meta.json temp path', () => {
expect(bridgeSource).toMatch(/\.tmp\.\$\{randomBytes\(8\)\.toString\('hex'\)\}/);
});
it('bridge-db.ts does not use Date.now() in any temp path', () => {
// Match Date.now() specifically in tmp-path contexts — not in unrelated code.
const tmpDateNow = bridgeSource.match(/\.tmp\.\$\{Date\.now\(\)\}/g) ?? [];
it('bridge-db.ts opens meta.json tmp file via fsp.open(..., "wx", 0o600)', () => {
// O_EXCL via `'wx'` flag closes the symlink-race; explicit `0o600`
// mode closes the permissions exposure CodeQL's
// `isSecureMode` predicate inspects (low 6 bits must be zero).
// Both arguments are required to fully clear the
// `js/insecure-temporary-file` alert — flags alone are ignored by
// the analyzer, mode alone leaves the symlink window open.
expect(bridgeSource).toMatch(/fsp\.open\(tmp,\s*['"]wx['"],\s*0o600\)/);
});
it('bridge-db.ts does not use Date.now() in any active temp path', () => {
// Strip block AND line comments first so the historical
// "prior `${target}.tmp.${Date.now()}` shape." explanation does not
// register as an active call site. Block strip runs first so a future
// multi-line `/* ...Date.now()... */` doc comment is also handled.
const codeOnly = bridgeSource.replace(/\/\*[\s\S]*?\*\//g, '').replace(/\/\/[^\n]*/g, '');
const tmpDateNow = codeOnly.match(/\.tmp\.\$\{Date\.now\(\)\}/g) ?? [];
expect(tmpDateNow.length).toBe(0);
});
it('bridge-db.ts uses readdir-based cleanup for stale bridge tmp files', () => {
expect(bridgeSource).toMatch(/cleanStaleBridgeTmpFiles/);
expect(bridgeSource).toMatch(/readdir\(groupDir\)/);
expect(bridgeSource).toMatch(/startsWith\('bridge\.lbug\.tmp\.'\)/);
});
it('bridge-db.ts calls cleanStaleBridgeTmpFiles before openBridgeDb in writeBridge', () => {
// Ensure cleanup happens before the DB is opened with the new random path.
const cleanIdx = bridgeSource.indexOf('cleanStaleBridgeTmpFiles(groupDir)');
const openIdx = bridgeSource.indexOf('openBridgeDb(tmpPath)');
expect(cleanIdx).toBeGreaterThan(-1);
expect(openIdx).toBeGreaterThan(-1);
expect(cleanIdx).toBeLessThan(openIdx);
it('bridge-db.ts removes the mkdtemp staging directory in finally', () => {
// Whether writeBridge succeeds or throws, the random staging dir
// must be cleaned up — otherwise the group dir accumulates
// bridge-tmp-* directories. The removal is idempotent (force: true).
expect(bridgeSource).toMatch(
/fsp\.rm\(stagingDir,\s*\{[^}]*recursive:\s*true[^}]*force:\s*true/,
);
});
it('storage.ts imports randomBytes from node:crypto', () => {
expect(storageSource).toMatch(/import\s*\{[^}]*randomBytes[^}]*\}\s*from\s*'node:crypto'/);
});
it('storage.ts uses randomBytes for contracts.json temp path', () => {
expect(storageSource).toMatch(/\.tmp\.\$\{randomBytes\(8\)\.toString\('hex'\)\}/);
it('storage.ts uses tmpSuffix() helper backed by randomBytes', () => {
// The helper is a thin wrapper that DRYs the randomBytes call across
// multiple temp-path sites in this module. Its definition must use
// randomBytes, and the temp path must call it.
expect(storageSource).toMatch(/const\s+tmpSuffix\s*=.*randomBytes\(8\)\.toString\('hex'\)/);
expect(storageSource).toMatch(/\.tmp\.\$\{tmpSuffix\(\)\}/);
});
it('storage.ts does not use Date.now() in any temp path', () => {
const tmpDateNow = storageSource.match(/\.tmp\.\$\{Date\.now\(\)\}/g) ?? [];
it('storage.ts does not use Date.now() in any active temp path', () => {
// Same comment-strip trick as bridge-db.ts above (block + line).
const codeOnly = storageSource.replace(/\/\*[\s\S]*?\*\//g, '').replace(/\/\/[^\n]*/g, '');
const tmpDateNow = codeOnly.match(/\.tmp\.\$\{Date\.now\(\)\}/g) ?? [];
expect(tmpDateNow.length).toBe(0);
});
});

View file

@ -186,3 +186,55 @@ const x = 1;
expect(components).toEqual(['MyComponent']);
});
});
// ---------------------------------------------------------------------------
// Case-insensitive script-tag matching (CodeQL js/bad-tag-filter, PR #1330)
// ---------------------------------------------------------------------------
describe('extractVueScript — case-insensitive script-tag matching', () => {
it('extracts content from <SCRIPT> ... </SCRIPT> (uppercase)', () => {
// HTML tag names are case-insensitive per the spec; browsers and
// Vue's SFC parser accept any case. The extractor MUST mirror that
// — a strict lowercase regex would miss valid SFC content and
// re-open the CodeQL js/bad-tag-filter alert PR #1330 closed.
const vue = `<template>
<div>Hello</div>
</template>
<SCRIPT setup lang="ts">
const greeting = 'hi';
</SCRIPT>
`;
const result = extractVueScript(vue);
expect(result).not.toBeNull();
expect(result!.scriptContent).toContain("const greeting = 'hi'");
});
it('extracts content from mixed-case <Script> ... </Script>', () => {
const vue = `<template>
<div>Hello</div>
</template>
<Script lang="ts">
export default { name: 'Mixed' };
</Script>
`;
const result = extractVueScript(vue);
expect(result).not.toBeNull();
expect(result!.scriptContent).toContain("name: 'Mixed'");
});
it('handles whitespace AND uppercase together: </SCRIPT >', () => {
const vue = `<template>
<div>Hi</div>
</template>
<SCRIPT setup>
const x = 1;
</SCRIPT >
`;
const result = extractVueScript(vue);
expect(result).not.toBeNull();
expect(result!.scriptContent).toContain('const x = 1');
});
});