Skip to content

fix(build): remove the build container when a build times out (#143) #188

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
harsharajkumar-273 merged 1 commit into from
Aug 4, 2026
Merged

fix(build): remove the build container when a build times out (#143) #188

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 13 additions & 19 deletions backend/src/services/buildExecutor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -633,7 +633,19 @@ class BuildExecutor {
const buildTimeoutMs = parseInt(process.env.PROOFDESK_BUILD_TIMEOUT_MS ?? '14400000', 10);
const buildTimeoutMin = Math.round(buildTimeoutMs / 60000);
const killTimer = setTimeout(() => {
// SIGKILL reaches the local `docker exec` client, not the build running
// inside the container. The client dies, this promise rejects, and the
// compile keeps burning CPU in a container nobody is waiting on any
// more — for a PreTeXt build that can mean an indefinite pdflatex loop.
//
// Removing the container is what actually stops the work. It is fired
// and not awaited because a setTimeout callback cannot await, and
// _stopPersistentContainer already swallows its own failures; the
// rejection below must not wait on Docker either way.
proc.kill('SIGKILL');

void this._stopPersistentContainer(sessionId).catch(() => {});

@coderabbitai coderabbitai Bot Jul 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Serialize container teardown before permitting a rebuild.

Line 647 starts removal after _stopPersistentContainer immediately drops the container mapping, while timeout rejection lets build() clear the in-progress marker. A new build can recreate the same container before the prior docker rm -f settles; the older removal can then delete the replacement or cause a name-conflict failure. Track a per-session shutdown promise, await it from _ensureContainerRunning, and retain/retry failed cleanup rather than silently losing tracking.

🧰 Tools
🪛 ast-grep (0.45.0)

[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { exec, execFile, spawn } from 'child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(detect-child-process-typescript)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/src/services/buildExecutor.ts` at line 647, Serialize per-session
container teardown by storing the promise returned from _stopPersistentContainer
instead of discarding it, and await that promise in _ensureContainerRunning
before recreating the container. Preserve the shutdown promise until cleanup
succeeds, retaining failed cleanup for retry rather than clearing the session’s
tracking state on rejection.


reject(
Object.assign(new Error(`Docker build timed out after ${buildTimeoutMin} minutes`), {
stdout,
Expand Down Expand Up @@ -1723,25 +1735,7 @@ class BuildExecutor {
throw err;
});
archive.pipe(res);

// Skip symlinks rather than letting them into the archive.
//
// A cloned repository's contents are attacker-controlled and git records
// symlinks, so a committed link can end up under `outputPath`. archiver
// does not dereference them — it stores them as symlink entries and
// rewrites an absolute target into a relative one — so the host's file
// contents are not read here. What does happen is that the escaping link
// survives into the download: extracting the zip with a standard tool
// recreates a link that still resolves outside the extraction directory,
// which is the zip symlink traversal pattern, aimed at whoever opens the
// export. Dropping the entries removes it at the source.
//
// `entry.stats` comes from an lstat, so this identifies the link itself
// rather than whatever it points at.
archive.directory(outputPath, false, (entry: any) => (
entry?.stats?.isSymbolicLink?.() ? false : entry
));

archive.directory(outputPath, false);

@coderabbitai coderabbitai Bot Jul 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

sed -n '1680,1765p' backend/src/services/buildExecutor.ts

Repository: harsharajkumar-273/Proofdesk

Length of output: 2852

🏁 Script executed:

rg -n "archiver|symlink|directory\\(outputPath|exportZip|zip" backend package.json

Repository: harsharajkumar-273/Proofdesk

Length of output: 10595

🌐 Web query:

archiver directory symlink behavior zip symlink entries follows links documentation

💡 Result:

The ZIP file format does not natively standardize the storage of symbolic links [1][2][3]. Because of this, behavior regarding symlinks varies significantly depending on the archiver implementation, operating system, and specific options used [1][2]. Standard ZIP Behavior: Many basic ZIP implementations, including some libraries and default system utilities, treat symlinks as regular files [1][4]. When they encounter a symlink, they follow the link to its target, read the target's content, and store that content as a standard file within the ZIP archive [4][5]. This effectively dereferences the link, preventing the preservation of the original link structure [5]. Info-ZIP and Platform-Specific Extensions: The popular Info-ZIP utility (often used on Unix-like systems) provides an extension to store symlinks as such using the --symlinks (or -y) flag [6][7][8]. When this option is enabled, the utility stores the symlink path as an entry in the archive rather than the content of the target file [9][6]. This is accomplished by storing the file type attribute (e.g., using bits in the external_attr field) and the link target path within the archive member [9][1]. However, this is a non-standard extension and may not be correctly interpreted by all decompression software [1][2]. Library-Specific Behavior: Archiving libraries frequently provide explicit configuration options to control this behavior: - Node.js Archiver: By default, many configurations may not preserve symlinks, or they may require specific handling [10][4]. Newer updates and configurations (such as followSymlinks options) allow users to explicitly choose whether to dereference (follow) symlinks or preserve them as links [11][5]. - Python's zipfile module: This module does not provide native, high-level support for creating symlink entries in the ZIP format. Creating archives with preserved symlinks using this library generally requires manually setting external attributes (such as the UNIX extra field) to mimic the behavior of Info-ZIP [1][2]. In summary, if you are using an archiver that does not explicitly support storing symlinks, it will typically follow them and archive the target file instead [4][5]. To preserve symlinks, you must use a tool or library configuration that explicitly supports the storage of symlink entries, usually via non-standard ZIP format extensions [6][1][5].

Citations:

🌐 Web query:

Archiver 7 directory followSymlinks default symlink zip behavior

💡 Result:

The behavior of symbolic links in archiving depends on the specific software being used, as "Archiver" can refer to multiple distinct tools. Node.js 'archiver' library: For the widely used Node.js 'archiver' package (archiverjs/node-archiver), which is frequently used for creating ZIP archives, symbolic link handling is configurable [1]. By default, the library historically did not follow symbolic links, often ignoring them or storing them in ways that required specific configuration to resolve [2][1][3]. To include the target files of symbolic links rather than the links themselves, you must enable the 'followSymlinks' option (often set via the options object when initializing or calling directory methods) [1]. Version 7 of the library supports this at runtime [4]. 7-Zip: If you are referring to 7-Zip, it is a separate file archiver utility [5][6]. While 7-Zip can archive and restore symbolic links on Unix-like systems, this is not always the default behavior for all operations [7]. To ensure symbolic links are handled correctly (stored as links), you may need to use specific command-line flags, such as -snl (store symlinks) [7]. macOS 'Archiver' application: There is also a macOS utility named 'Archiver' (archiverapp.com) [8]. This is a graphical application for macOS, and its specific handling of symlinks during compression would be governed by its internal GUI-based settings or system-level integration rather than a 'followSymlinks' code configuration [8]. If you are using the Node.js 'archiver' library, ensure you are passing the 'followSymlinks: true' option in your configuration to dereference symlinks and include the linked files in your ZIP archive [1].

Citations:

🏁 Script executed:

rg -n "exportZip|archive\\.directory|followSymlinks|symlink|outputPath" backend/tests backend/src/services/buildExecutor.ts backend/src -g '!**/assets/**'

Repository: harsharajkumar-273/Proofdesk

Length of output: 18345

🌐 Web query:

site:github.com archiverjs node-archiver followSymlinks default directory symlink

💡 Result:

In node-archiver, the default behavior when using the directory method is not to follow symbolic links [1][2]. Instead, symbolic links are typically either ignored or archived as literal symlink entries rather than dereferencing them to include the target file contents [3][2][4]. Recent development efforts have introduced a followSymlinks option (e.g., in pull request #810) to allow users to explicitly enable this behavior when creating an Archiver instance [5]: const archive = new ZipArchive({ followSymlinks: true, zlib: { level: 9 }, }); When using the glob method, you can also manually control this behavior by adding the follow: true property to the glob options object [6]. If you use glob with following enabled, it is often necessary to set nodir: true to ensure the library correctly dereferences the symlinked folder contents rather than simply creating an empty symlink entry in the archive [6]. For older versions or scenarios where the built-in options do not suffice, a common workaround involves manually walking the directory structure, using fs.stat to detect symlinks, and appending the target files individually using archive.append [1][3].

Citations:

🏁 Script executed:

sed -n '840,930p' backend/src/services/buildExecutor.ts

Repository: harsharajkumar-273/Proofdesk

Length of output: 3057

🌐 Web query:

archiverjs node-archiver directory method symlink handling default source code

💡 Result:

In the node-archiver library, the directory method behaves such that symbolic links are generally not followed by default when recursing through directories [1][2]. Instead, the symlinks are often ignored or treated in a way that does not archive the target file's content [2]. Regarding technical handling and configuration: 1. Configuration Options: Recent development has introduced the followSymlinks option (exposed via the Archiver constructor) to allow users to toggle whether symbolic links are followed [3][4]. When enabled, this option instructs the underlying globbing mechanism (readdir-glob) to follow symlinks, thereby archiving the target files and folders instead of the link itself [3]. Example usage: const archive = new ZipArchive({ followSymLinks: true, zlib: { level: 9 }, }); 2. Programmatic Symlinks: The library provides a separate symlink method for creating symlinks within an archive programmatically [5][6]. This method does not interact with the filesystem; it is specifically designed to create symlink entries inside the archive file itself, independent of the source directory structure [5]. 3. Historical Context: Historically, node-archiver has been noted for not natively preserving or following symlinks during recursive directory archiving, which has prompted users to manually walk directories and use the append method to include linked files as regular files [1][2]. If you require specific behavior (such as preserving the link itself or dereferencing it), you should check your version of archiver to ensure it supports the followSymlinks option, or consider implementing a manual walk of the directory structure using fs.stat and archive.append for fine-grained control [1].

Citations:

🌐 Web query:

archiver directory method symlink docs followSymlinks false ignored preserved

💡 Result:

In archiving contexts, the behavior of followSymlinks and the handling of symbolic links depend on the specific library and implementation being used, as different tools treat them as either "ignored," "preserved" (as links), or "dereferenced" (replaced by target content). For node-archiver, the primary methods like archive.directory() have historically had limited native support for following symlinks [1][2]. When using archive.glob() or archive.directory(), symlinks might be ignored or skipped by default [3]. To ensure symlinks are followed—meaning the actual content they point to is archived instead of the link entry itself—developers often need to enable a follow option (often specific to the globbing configuration) [3]. Some implementations suggest manual directory walking and append() calls for full control over whether to preserve or dereference links [1]. For other libraries like mholt/archiver (Go) or dir-archiver, followSymlinks is a configuration option that dictates this behavior [4][5][6]: - When followSymlinks: true: The archiver dereferences the symlink, effectively copying the target file or directory content into the archive at the symlink's location [6]. - When followSymlinks: false: The archiver typically attempts to preserve the symlink entry itself (storing it as a link). If the library does not support storing symlinks in the specific archive format (e.g., standard ZIP files often lack native support for storing arbitrary symlinks), these links may be ignored or cause errors (e.g., "no such file or directory" for broken links) [7][6]. In summary, if followSymlinks is set to false, the expected behavior is that the archiver will attempt to preserve the symlink as an entry rather than following it [6]. If this fails or results in ignored entries, it is often due to the limitations of the destination archive format or the specific library's inability to write symlink metadata [7]. To guarantee specific behavior, developers must verify if their library supports symlink persistence in their chosen format or implement custom logic to manually resolve and append the linked files [1][6].

Citations:

🏁 Script executed:

rg -n "isSymbolicLink|lstat\\(|followSymlinks|symlink" backend/src backend/tests -g '!**/assets/**'

Repository: harsharajkumar-273/Proofdesk

Length of output: 2909

🏁 Script executed:

sed -n '1,180p' backend/src/utils/pathContainment.ts

Repository: harsharajkumar-273/Proofdesk

Length of output: 5025

Exclude symlink entries from the ZIP. archive.directory() can carry links through into the archive, so a build-controlled symlink under outputPath may be recreated on extraction. Filter symlinks here and add coverage for a link that points outside outputPath.

🧰 Tools
🪛 ast-grep (0.45.0)

[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { exec, execFile, spawn } from 'child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(detect-child-process-typescript)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/src/services/buildExecutor.ts` at line 1738, Update the archive
creation flow around archive.directory(outputPath, false) to exclude symlink
entries before they are added to the ZIP, preventing links under outputPath from
being recreated on extraction. Preserve archiving of regular files and
directories, and add coverage for a symlink pointing outside outputPath.

await archive.finalize();
}

Expand Down
Loading