Skip to content

Commit 73b1fef

Browse files
committed
Stream large files instead of loading into memory
- addresses latest review feedback for PR #119 - search-ql-code: check file size via lstatSync before reading; stream large files (>5 MB) line-by-line instead of skipping them - evaluator-log-parser: replace readFileSync with streaming async generator (createReadStream + readline) for brace-depth JSON parsing; parseEvaluatorLog now reads the file once instead of twice - profile-codeql-query: convert local parser to streaming with Map-based lookups instead of O(n) events.find() - database-copier: use lstat in removeLockFiles to skip symlinks; throw on fatal mkdir failures for proper fallback in EnvironmentBuilder - Validate contextLines/maxResults with schema bounds and clamping - Add environment-builder test for syncAll-throws fallback
1 parent 5fb6146 commit 73b1fef

10 files changed

Lines changed: 745 additions & 397 deletions

extensions/vscode/src/bridge/database-copier.ts

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { existsSync } from 'fs';
2-
import { cp, mkdir, readdir, rm, stat, unlink } from 'fs/promises';
2+
import { cp, lstat, mkdir, readdir, rm, stat, unlink } from 'fs/promises';
33
import { join } from 'path';
44
import type { Logger } from '../common/logger';
55

@@ -34,10 +34,10 @@ export class DatabaseCopier {
3434
try {
3535
await mkdir(this.destinationBase, { recursive: true });
3636
} catch (err) {
37-
this.logger.error(
38-
`Failed to create managed database directory ${this.destinationBase}: ${err instanceof Error ? err.message : String(err)}`,
39-
);
40-
return [];
37+
const msg =
38+
`Failed to create managed database directory ${this.destinationBase}: ${err instanceof Error ? err.message : String(err)}`;
39+
this.logger.error(msg);
40+
throw new Error(msg, { cause: err });
4141
}
4242

4343
const copied: string[] = [];
@@ -137,8 +137,8 @@ async function isCodeQLDatabase(dirPath: string): Promise<boolean> {
137137

138138
/**
139139
* Recursively remove all `.lock` files under the given directory.
140-
* These are empty sentinel files created by the CodeQL query server in
141-
* `<dataset>/default/cache/.lock`.
140+
* Uses lstat to avoid following symlinks, keeping deletion scoped to
141+
* the copied database tree.
142142
*/
143143
async function removeLockFiles(dir: string): Promise<void> {
144144
let entries: string[];
@@ -151,7 +151,11 @@ async function removeLockFiles(dir: string): Promise<void> {
151151
for (const entry of entries) {
152152
const fullPath = join(dir, entry);
153153
try {
154-
const st = await stat(fullPath);
154+
const st = await lstat(fullPath);
155+
// Skip symlinks to avoid traversing outside the database directory
156+
if (st.isSymbolicLink()) {
157+
continue;
158+
}
155159
if (st.isDirectory()) {
156160
await removeLockFiles(fullPath);
157161
} else if (entry === '.lock') {

extensions/vscode/test/bridge/environment-builder.test.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,15 @@ describe('EnvironmentBuilder', () => {
206206
vscode.workspace.getConfiguration = originalGetConfig;
207207
});
208208

209+
it('should fall back to source dirs when syncAll throws', async () => {
210+
mockCopier.syncAll.mockRejectedValue(new Error('Failed to create managed database directory'));
211+
builder.invalidate();
212+
const env = await builder.build();
213+
expect(env.CODEQL_DATABASES_BASE_DIRS).toBe(
214+
['/mock/global-storage/GitHub.vscode-codeql', '/mock/workspace-storage/ws-123/GitHub.vscode-codeql'].join(delimiter),
215+
);
216+
});
217+
209218
it('should append user-configured dirs to CODEQL_QUERY_RUN_RESULTS_DIRS', async () => {
210219
const vscode = await import('vscode');
211220
const originalGetConfig = vscode.workspace.getConfiguration;

0 commit comments

Comments
 (0)