Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion packages/config/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ This is an internal package that handles configuration loading, schema validatio
| Export | Description |
|--------|-------------|
| `parseJsonc(content)` | Parse JSON with comments |
| `substituteInObject(obj)` | Substitute `{env:VAR}` and `{file:path}` |
| `substituteInObject(obj, rootDir)` | Substitute `{env:VAR}` and `{file:path}` (file reads confined to `rootDir`) |
| `deepMerge(target, source)` | Deep merge objects |
| `mergeGitConfig(top, pkg)` | Merge git config layers |

Expand Down
4 changes: 3 additions & 1 deletion packages/config/src/load.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,9 @@ function loadConfigFile(configPath: string): ReleaseKitConfig {
try {
const content = fs.readFileSync(configPath, 'utf-8');
const parsed = parseJsonc(content);
const substituted = substituteInObject(parsed);
// Confine {file:} substitution to the config file's directory (the repo root) so a config can't
// read files outside the tree.
const substituted = substituteInObject(parsed, path.dirname(configPath));
assertNoRemovedReleaseNotesFields(substituted);
assertNoRemovedVersionFields(substituted);
assertNoRemovedTopLevelFields(substituted);
Expand Down
35 changes: 28 additions & 7 deletions packages/config/src/substitute.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,25 @@
import * as fs from 'node:fs';
import * as os from 'node:os';
import * as path from 'node:path';
import { isPathWithinRoot } from '@releasekit/core';
import { ConfigError } from './errors.js';

const MAX_INPUT_LENGTH = 10000;

export function substituteVariables(value: string): string {
/**
* Inline `{env:NAME}` and `{file:REL_PATH}` references in a config string.
*
* Trust boundary: substituted values flow into config fields that can surface in bot-authored
* output — tag templates, `standingPr.title`, changelog/notes text echoed into PR comments and
* releases — so the two read primitives are scoped differently:
* - `{env:NAME}` reads process env, the trusted-operator injection point for secrets (npm/LLM
* tokens). Returned verbatim; an unset variable resolves to an empty string.
* - `{file:REL_PATH}` is confined to `rootDir` (the config file's directory). A reference that
* escapes it — via `..` or an absolute path outside the tree — is rejected rather than read, so a
* config cannot exfiltrate files such as `~/.ssh/id_rsa` or `/etc/passwd`. An in-bounds but
* unreadable path resolves to an empty string.
*/
export function substituteVariables(value: string, rootDir: string = process.cwd()): string {
// Limit input length to prevent ReDoS attacks
if (value.length > MAX_INPUT_LENGTH) {
throw new Error(`Input too long: ${value.length} characters (max ${MAX_INPUT_LENGTH})`);
Expand All @@ -21,10 +36,16 @@ export function substituteVariables(value: string): string {
});

result = result.replace(filePattern, (_, filePath: string) => {
const expandedPath = filePath.startsWith('~') ? path.join(os.homedir(), filePath.slice(1)) : filePath;
const resolved = path.resolve(rootDir, filePath);
if (!isPathWithinRoot(rootDir, resolved)) {
throw new ConfigError(
`{file:${filePath}} resolves outside the config directory (${path.resolve(rootDir)}); ` +
'config file references are confined to the repository. Use {env:NAME} for secrets kept outside the repo.',
);
}

try {
return fs.readFileSync(expandedPath, 'utf-8').trim();
return fs.readFileSync(resolved, 'utf-8').trim();
} catch {
return '';
}
Expand All @@ -35,9 +56,9 @@ export function substituteVariables(value: string): string {

const SOLE_REFERENCE_PATTERN = /^\{(?:env|file):[^}]+\}$/;

export function substituteInObject<T>(obj: T): T {
export function substituteInObject<T>(obj: T, rootDir: string = process.cwd()): T {
if (typeof obj === 'string') {
const result = substituteVariables(obj);
const result = substituteVariables(obj, rootDir);
// When the entire string was a single {env:…} or {file:…} reference that
// resolved to nothing, return undefined so downstream ?? fallbacks work.
if (result === '' && SOLE_REFERENCE_PATTERN.test(obj)) {
Expand All @@ -47,13 +68,13 @@ export function substituteInObject<T>(obj: T): T {
}

if (Array.isArray(obj)) {
return obj.map((item) => substituteInObject(item)) as T;
return obj.map((item) => substituteInObject(item, rootDir)) as T;
}

if (obj && typeof obj === 'object') {
const result: Record<string, unknown> = {};
for (const [key, value] of Object.entries(obj)) {
result[key] = substituteInObject(value);
result[key] = substituteInObject(value, rootDir);
}
return result as T;
}
Expand Down
40 changes: 27 additions & 13 deletions packages/config/test/unit/substitute.spec.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as fs from 'node:fs';
import * as os from 'node:os';
import * as path from 'node:path';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { ConfigError } from '../../src/errors.js';
import { loadAuth, saveAuth, substituteInObject, substituteVariables } from '../../src/substitute.js';

vi.mock('node:fs', () => ({
Expand All @@ -12,6 +12,7 @@ vi.mock('node:fs', () => ({
}));

const mockedFs = vi.mocked(fs);
const fileRoot = path.resolve('/repo');

describe('substituteVariables', () => {
const originalEnv = process.env;
Expand Down Expand Up @@ -45,30 +46,38 @@ describe('substituteVariables', () => {
expect(result).toBe('No variables here');
});

it('should substitute {file:PATH} with file contents', () => {
it('should substitute {file:PATH} with file contents from within the confinement root', () => {
mockedFs.readFileSync.mockReturnValue(' file contents \n');
const result = substituteVariables('File: {file:/path/to/file.txt}');
const result = substituteVariables('File: {file:secret.txt}', fileRoot);
expect(result).toBe('File: file contents');
expect(mockedFs.readFileSync).toHaveBeenCalledWith('/path/to/file.txt', 'utf-8');
expect(mockedFs.readFileSync).toHaveBeenCalledWith(path.join(fileRoot, 'secret.txt'), 'utf-8');
});

it('should expand ~ to home directory in file path', () => {
mockedFs.readFileSync.mockReturnValue('contents');
substituteVariables('{file:~/secrets/api-key}');
expect(mockedFs.readFileSync).toHaveBeenCalledWith(path.join(os.homedir(), 'secrets/api-key'), 'utf-8');
it('should resolve a {file:} reference relative to the confinement root', () => {
mockedFs.readFileSync.mockReturnValue('nested');
substituteVariables('{file:config/token}', fileRoot);
expect(mockedFs.readFileSync).toHaveBeenCalledWith(path.join(fileRoot, 'config', 'token'), 'utf-8');
});

it('should return empty string for unreadable file', () => {
it('should reject a {file:} reference that escapes the root via ..', () => {
expect(() => substituteVariables('{file:../../etc/passwd}', fileRoot)).toThrow(ConfigError);
});

it('should reject an absolute {file:} reference outside the root', () => {
expect(() => substituteVariables('{file:/etc/passwd}', fileRoot)).toThrow(/outside the config directory/);
});

it('should return empty string for an in-bounds unreadable file', () => {
mockedFs.readFileSync.mockImplementation(() => {
throw new Error('ENOENT');
});
const result = substituteVariables('{file:/nonexistent}');
const result = substituteVariables('{file:missing.txt}', fileRoot);
expect(result).toBe('');
});

it('should substitute mixed env and file variables', () => {
mockedFs.readFileSync.mockReturnValue('file-content');
const result = substituteVariables('{env:TEST_VAR} and {file:/path/to/file}');
const result = substituteVariables('{env:TEST_VAR} and {file:data.txt}', fileRoot);
expect(result).toBe('test-value and file-content');
});
});
Expand Down Expand Up @@ -167,11 +176,16 @@ describe('substituteInObject', () => {
mockedFs.readFileSync.mockImplementation(() => {
throw new Error('ENOENT');
});
const obj = { secret: '{file:/nonexistent/path}' };
const result = substituteInObject(obj);
const obj = { secret: '{file:nonexistent/path}' };
const result = substituteInObject(obj, fileRoot);
expect(result.secret).toBeUndefined();
});

it('should reject a {file:} reference that escapes the confinement root', () => {
const obj = { secret: '{file:../../../etc/passwd}' };
expect(() => substituteInObject(obj, fileRoot)).toThrow(ConfigError);
});

it('should substitute in deeply nested structures', () => {
const obj = {
level1: {
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export {
shouldMatchPackageTargets,
shouldProcessPackage,
} from './packageUtils.js';
export { isPathWithinRoot } from './paths.js';
export { type PrerequisiteResolution, resolvePrerequisites } from './prerequisites.js';
export {
extractSelectionRegion,
Expand Down
23 changes: 23 additions & 0 deletions packages/core/src/paths.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import * as path from 'node:path';

/**
* True when `target` resolves to `root` itself or a path nested within it.
*
* Used to confine config-driven filesystem access — manifest write targets (`version.cargo.paths` /
* `version.pub.paths`), `npm.copyFiles` destinations, and `{file:}` reads — to the repository root so
* a malicious or mistaken config cannot drive reads/writes outside the tree via `..` or an absolute
* path. Both paths are resolved to absolute form first, so a relative `target` is interpreted
* against `process.cwd()` unless the caller has already resolved it against the intended base.
*/
export function isPathWithinRoot(root: string, target: string): boolean {
const resolvedRoot = path.resolve(root);
const resolvedTarget = path.resolve(target);
if (resolvedTarget === resolvedRoot) {
return true;
}
const relative = path.relative(resolvedRoot, resolvedTarget);
// The equal case already returned, so `relative` is non-empty here. `..` / `../…` catch parent
// escapes; isAbsolute catches a Windows cross-drive target (path.relative returns an absolute path
// when root and target are on different drives).
return relative !== '..' && !relative.startsWith(`..${path.sep}`) && !path.isAbsolute(relative);
}
45 changes: 45 additions & 0 deletions packages/core/test/unit/paths.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import * as path from 'node:path';
import { describe, expect, it } from 'vitest';
import { isPathWithinRoot } from '../../src/paths.js';

const root = path.resolve('/repo');

describe('isPathWithinRoot', () => {
it('should treat the root itself as within the root', () => {
expect(isPathWithinRoot(root, root)).toBe(true);
});

it('should accept a direct child of the root', () => {
expect(isPathWithinRoot(root, path.join(root, 'packages', 'foo', 'Cargo.toml'))).toBe(true);
});

it('should accept a relative path that resolves inside the root', () => {
expect(isPathWithinRoot(root, path.join(root, 'packages', '..', 'crates', 'foo'))).toBe(true);
});

it('should reject a parent-directory escape', () => {
expect(isPathWithinRoot(root, path.join(root, '..', 'other-repo', 'crate'))).toBe(false);
});

it('should reject a deeply nested escape that climbs above the root', () => {
expect(isPathWithinRoot(root, path.join(root, 'packages', 'foo', '..', '..', '..', 'secrets'))).toBe(false);
});

it('should reject the root parent directory', () => {
expect(isPathWithinRoot(root, path.dirname(root))).toBe(false);
});

it('should reject an absolute path outside the root', () => {
expect(isPathWithinRoot(root, path.resolve('/etc/passwd'))).toBe(false);
});

it('should accept a sibling whose name merely starts with the escape sequence', () => {
// A child literally named "..foo" is inside the root — it must not be mistaken for a `..` escape.
expect(isPathWithinRoot(root, path.join(root, '..foo'))).toBe(true);
});

it('should reject a sibling directory sharing the root name prefix', () => {
// "/repo-evil" shares the "/repo" prefix as a string but is not nested within "/repo".
expect(isPathWithinRoot(root, path.resolve('/repo-evil', 'crate'))).toBe(false);
});
});
11 changes: 10 additions & 1 deletion packages/publish/src/stages/prepare.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as fs from 'node:fs';
import * as path from 'node:path';
import { debug, info } from '@releasekit/core';
import { debug, info, isPathWithinRoot } from '@releasekit/core';
import { createPublishError, PublishErrorCode } from '../errors/index.js';
import type { PipelineContext } from '../types.js';
import { syncCargoLockfile, updateCargoVersion } from '../utils/cargo.js';
Expand All @@ -17,6 +17,15 @@ export async function runPrepareStage(ctx: PipelineContext): Promise<void> {
const src = path.resolve(cwd, file);
const dest = path.join(pkgDir, file);

// Confine both the read source and the write destination to the repo root — a `../` or
// absolute copyFiles entry would otherwise read or write outside the tree.
if (!isPathWithinRoot(cwd, src) || !isPathWithinRoot(cwd, dest)) {
throw createPublishError(
PublishErrorCode.FILE_COPY_ERROR,
`Refusing to copy '${file}': it resolves outside the repository root (${cwd})`,
);
}

if (!fs.existsSync(src)) {
debug(`Source file not found, skipping copy: ${src}`);
continue;
Expand Down
24 changes: 24 additions & 0 deletions packages/publish/test/unit/stages/prepare.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -225,4 +225,28 @@ describe('prepare stage', () => {
await runPrepareStage(ctx);
expect(fs.readFileSync(path.join(dir, 'LICENSE'), 'utf-8')).toBe('MIT License');
});

it('should reject a copyFiles entry that escapes the repository root', async () => {
const dir = createTmpDir();
const pkgDir = path.join(dir, 'packages', 'foo');
fs.mkdirSync(pkgDir, { recursive: true });
fs.writeFileSync(path.join(pkgDir, 'package.json'), '{"name":"foo","version":"1.0.0"}');

const config = getDefaultConfig();
config.npm.enabled = true;
config.npm.copyFiles = ['../../escape.txt'];

const ctx = createContext({
cwd: dir,
config,
input: {
dryRun: false,
updates: [{ packageName: 'foo', newVersion: '1.1.0', filePath: 'packages/foo/package.json' }],
changelogs: [],
tags: [],
},
});

await expect(runPrepareStage(ctx)).rejects.toThrow(/outside the repository root/);
});
});
25 changes: 18 additions & 7 deletions packages/version/src/core/versionStrategies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { getLatestTag, getLatestTagForPackage } from '../git/tagsAndBranches.js'
import { updatePackageVersion } from '../package/packageManagement.js';
import { PackageProcessor } from '../package/packageProcessor.js';
import type { Config } from '../types.js';
import { resolveConfinedManifestPath } from '../utils/confinedPath.js';
import { deriveBaselineTagPrefix, formatCommitMessage, formatTag, formatVersionPrefix } from '../utils/formatting.js';
import {
addBaselineTag,
Expand Down Expand Up @@ -65,9 +66,17 @@ function shouldProcessPackage(pkg: Package, config: Config): boolean {
* @param packageDir - The directory containing the package
* @param version - The version to update to
* @param cargoConfig - The cargo configuration from config
* @param dryRun - When true, log the intended writes without touching disk
* @param repoRoot - Confinement boundary for config-driven `cargo.paths` writes
* @returns Array of Cargo.toml file paths that were updated
*/
function updateCargoFiles(packageDir: string, version: string, cargoConfig: Config['cargo'], dryRun = false): string[] {
function updateCargoFiles(
packageDir: string,
version: string,
cargoConfig: Config['cargo'],
dryRun = false,
repoRoot: string = process.cwd(),
): string[] {
const updatedFiles: string[] = [];

// Check if Cargo.toml handling is enabled (default to true if not specified)
Expand All @@ -92,7 +101,7 @@ function updateCargoFiles(packageDir: string, version: string, cargoConfig: Conf
if (cargoPaths && cargoPaths.length > 0) {
// If paths are specified, only include those Cargo.toml files
for (const cargoPath of cargoPaths) {
const resolvedCargoPath = path.resolve(packageDir, cargoPath, 'Cargo.toml');
const resolvedCargoPath = resolveConfinedManifestPath(repoRoot, packageDir, cargoPath, 'Cargo.toml');
if (fs.existsSync(resolvedCargoPath)) {
stageCargo(resolvedCargoPath);
}
Expand Down Expand Up @@ -240,7 +249,7 @@ export function createSyncStrategy(config: Config): StrategyFunction {

// Root Cargo.toml handling is independent of npm (updateCargoFiles honors cargo.enabled),
// so it runs whether or not npm versioning is disabled — matching the per-package loop below.
const rootCargoFiles = updateCargoFiles(packages.root, nextVersion, config.cargo, dryRun);
const rootCargoFiles = updateCargoFiles(packages.root, nextVersion, config.cargo, dryRun, repoRoot);
files.push(...rootCargoFiles);
// Record 'root' when only the root Cargo.toml was versioned (npm disabled, or no root
// package.json). In a single-package repo where packages.root === packages[0].dir the loop
Expand Down Expand Up @@ -286,7 +295,7 @@ export function createSyncStrategy(config: Config): StrategyFunction {
}

// Handle Cargo.toml files for this package
const pkgCargoFiles = updateCargoFiles(pkg.dir, nextVersion, config.cargo, dryRun);
const pkgCargoFiles = updateCargoFiles(pkg.dir, nextVersion, config.cargo, dryRun, repoRoot);
files.push(...pkgCargoFiles);

// Track by name when only the Cargo.toml was updated: a pure-Rust package (no package.json),
Expand Down Expand Up @@ -534,6 +543,7 @@ export function createSingleStrategy(config: Config): StrategyFunction {
}

const pkgPath = pkg.dir;
const repoRoot = packages.root ?? process.cwd();
const formattedPrefix = formatVersionPrefix(versionPrefix || 'v');

// Try to get the latest tag specific to this package first
Expand Down Expand Up @@ -669,7 +679,7 @@ export function createSingleStrategy(config: Config): StrategyFunction {
}

// Handle Cargo.toml files for this package
const cargoFiles = updateCargoFiles(pkgPath, nextVersion, config.cargo, dryRun);
const cargoFiles = updateCargoFiles(pkgPath, nextVersion, config.cargo, dryRun, repoRoot);
filesToCommit.push(...cargoFiles);

log(`Updated package ${packageName} to version ${nextVersion}`, 'success');
Expand Down Expand Up @@ -754,8 +764,9 @@ export function createAsyncStrategy(config: Config): StrategyFunction {

log(`Processing ${packagesToProcess.length} packages`, 'info');

// 2. Process packages with PackageProcessor
const result = await packageProcessor.processPackages(packagesToProcess);
// 2. Process packages with PackageProcessor. Pass the discovered workspace root so
// config-driven cargo.paths / pub.paths writes are confined to the repository.
const result = await packageProcessor.processPackages(packagesToProcess, packages.root ?? process.cwd());

// 3. Report results
if (result.updatedPackages.length === 0) {
Expand Down
Loading
Loading