Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jan 14, 2026

The changeset script had a Time-of-Check-Time-of-Use (TOCTOU) vulnerability where file existence checks (fs.existsSync) preceded file operations, creating an exploitable window for file manipulation attacks.

Changes

  • updateChangelog function: Replace existsSync + readFileSync/writeFileSync pattern with file descriptor-based operations using fs.openSync
  • readChangesets function: Remove existsSync directory check, directly attempt readdirSync with error handling

Implementation

Before:

if (fs.existsSync(changelogPath)) {
  existingContent = fs.readFileSync(changelogPath, "utf8");
}
fs.writeFileSync(changelogPath, updatedContent, "utf8");

After:

try {
  fd = fs.openSync(changelogPath, fs.constants.O_RDONLY);
  existingContent = fs.readFileSync(fd, "utf8");
  fs.closeSync(fd);
} catch (error) {
  if (error.code === "ENOENT") {
    existingContent = "# Changelog\n\n...";
  } else {
    throw error;
  }
}

fd = fs.openSync(changelogPath, fs.constants.O_WRONLY | fs.constants.O_CREAT | fs.constants.O_TRUNC, 0o644);
fs.writeFileSync(fd, updatedContent, "utf8");
fs.closeSync(fd);

File descriptors eliminate the check-use gap, making file operations atomic. Error handling via ENOENT catches preserves original behavior.

Original prompt

Please fix this security alert on scripts/changeset.js:463

Often it is necessary to check the state of a file before using it. These checks usually take a file name to be checked, and if the check returns positively, then the file is opened or otherwise operated upon.

However, in the time between the check and the operation, the underlying file referenced by the file name could be changed by an attacker, causing unexpected behavior.

Recommendation
Use file descriptors instead of file names whenever possible.

Example
The following example shows a case where the code checks whether a file inside the /tmp/ folder exists, and if it doesn't, the file is written to that location.

const fs = require("fs");
const os = require("os");
const path = require("path");

const filePath = path.join(os.tmpdir(), "my-temp-file.txt");

if (!fs.existsSync(filePath)) {
fs.writeFileSync(filePath, "Hello", { mode: 0o600 });
}
However, in a multi-user environment the file might be created by another user between the existence check and the write.

This can be avoided by using fs.open to get a file descriptor, and then use that file descriptor in the write operation.

const fs = require("fs");
const os = require("os");
const path = require("path");

const filePath = path.join(os.tmpdir(), "my-temp-file.txt");

try {
const fd = fs.openSync(filePath, fs.O_CREAT | fs.O_EXCL | fs.O_RDWR, 0o600);

fs.writeFileSync(fd, "Hello");
} catch (e) {
// file existed
}
References
Wikipedia: Time-of-check to time-of-use.
The CERT Oracle Secure Coding Standard for C: FIO01-C. Be careful using functions that use file names for identification .
NodeJS: The FS module.
Common Weakness Enumeration: CWE-367.


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Replace fs.existsSync check with file descriptor-based operations to eliminate
Time-of-Check-Time-of-Use (TOCTOU) vulnerabilities in:
- updateChangelog function (line 463 - primary security issue)
- readChangesets function (line 114 - additional TOCTOU prevention)

Changes:
- Use fs.openSync with O_RDONLY for reading CHANGELOG.md
- Use fs.openSync with O_WRONLY | O_CREAT | O_TRUNC for writing CHANGELOG.md
- Properly close file descriptors after operations
- Handle ENOENT errors to maintain same behavior as existsSync
- Use try-catch for directory reading instead of existence check

This eliminates the window of vulnerability where an attacker could modify
files between existence checks and file operations.

Co-authored-by: eaftan <4733401+eaftan@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix security alert in changeset.js file Fix TOCTOU vulnerability in changeset.js file operations Jan 14, 2026
Copilot AI requested a review from eaftan January 14, 2026 22:51
@pelikhan
Copy link
Contributor

@copilot format, lint

Copy link
Contributor Author

Copilot AI commented Jan 14, 2026

@copilot format, lint

The code is already properly formatted and linted. Ran make fmt-cjs and make lint-cjs - all checks pass with no changes needed.

Copilot AI requested a review from pelikhan January 14, 2026 23:16
@pelikhan pelikhan marked this pull request as ready for review January 14, 2026 23:24
@pelikhan pelikhan merged commit ea48216 into main Jan 14, 2026
69 checks passed
@pelikhan pelikhan deleted the copilot/fix-security-alert-in-script branch January 14, 2026 23:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants