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
1 change: 1 addition & 0 deletions .changelog/next/fixed-issue-4293.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- [issue-4293] Workspace-root rejection logs now identify the rejected path and checked roots.
2 changes: 1 addition & 1 deletion server/lib/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ The barrel `server/lib/index.js` is a machine-checkable enumeration of every pub
| `uuid.js` | `v4()` thin wrapper over `crypto.randomUUID()`. |
| `versionUtils.js` | `compareSemver(a, b)` — semver ordering (-1/0/1) with pre-release precedence and build-metadata stripping. Shared by the self-update checker (`updateChecker.js`) and the local-LLM Ollama update detector (`localLlm.js`). Inputs must be `v`-stripped. |
| `workTracker.js` | `WORK_TRACKERS`/`CONCRETE_WORK_TRACKERS`/`DEFAULT_WORK_TRACKER`, `workTrackerLabel`, `hostToWorkTracker`, `isGithubHost` (GitHub-family host test — github.com + enterprise github.*; enterprise-aware replacement for the github.com-only `isGithub` gate), `githubRepoSpec(origin)` (host-qualified `HOST/OWNER/REPO` selector for `gh --repo`, or null for a non-GitHub origin — pairs the isGithubHost gate with the selector so prWatcher/branchReconcile/issueReconcile share one "resolvable GitHub repo" definition), `forgeCliForTracker`, `isFileTracker` (true when the tracker records work as repo files — PLAN.md — so an agent's proposal necessarily dirties the worktree; false for github/gitlab/jira), `trackerToClaimTaskType`, `hostFromOriginUrl` (subgroup-tolerant host parse), pure `resolveWorkTracker({configured,host})`, async `resolveAppWorkTracker(app)` — resolves a managed app's autonomous work source (PLAN.md / GitHub / GitLab / JIRA), defaulting `'auto'` to the git origin host. Async `resolveRepoForgeTarget(repoPath)` — the ONE definition of "which forge can we query for this checkout", returning `{ forge, fullName, repoSpec, apiHost }` (enterprise-aware `gh --repo` selector for GitHub; `repoSpec: null` for GitLab, which `glab` resolves from its cwd) or null for a non-forge origin; shared by `issueReconcile.js` and `appIssues.js`. Async `resolveAppForgeTarget(app, {repoPath})` — the composed `resolveAppWorkTracker` + `resolveRepoForgeTarget` for callers holding the managed-app record, returning `{ tracker, target }` with the app's github/gitlab pin threaded in as `preferredForge` (so a self-hosted forge on a hostname matching neither pattern still resolves); use this instead of re-threading the pin by hand. Also owns the `{trackerInstructions}` prompt block shared by the TRACKER-FILING task types (types that read the app read-only and deliver findings as tracker items, not a commit): `TRACKER_FILING_PRESETS` (per-task-type slug prefix / label / body requirements — `reference-watch`, `ux`, `repo-study`), `TRACKER_FILING_TASK_TYPES` (derived from the presets, so a gated type always has wording), and `formatTrackerInstructions(tracker, options)` which renders the plan/github/gitlab/jira block (reference-watch is the default option set, so a bare call stays byte-identical for it). Consumed by the `claim-work` router in `cosTaskGenerator.js`, `referenceRepos.js`, and `routes/apps.js`. |
| `workspaceRoots.js` | Shared allow-list for routes that take a caller-supplied filesystem path. `isWithinAllowedRoots(realPath)` is the single complete test — on Windows it also allows any lettered non-system drive (`D:\code`), which appears in no root, so never render `ALLOWED_WORKSPACE_ROOTS` as "the directories you may use". Also `ALLOWED_WORKSPACE_ROOTS` (defaults + `PORTOS_WORKSPACE_ROOTS`, split on the platform path delimiter — `;` on Windows — and symlink-resolved), `isWithinRoot(resolvedPath, root)` (separator-safe containment), and `WORKSPACE_ROOTS_CONFIGURED` (true when the operator set the env var — lets a permissive-by-default route like `routes/detect.js` opt into confinement). Defaults cover home plus wherever the platform mounts secondary volumes: `/tmp` + `/Users` + `/Volumes` + `/mnt` + `/media` + `/opt` on POSIX; home + the temp dir plus the non-system-drive rule on Windows, where those POSIX literals would resolve to whatever drive the process happens to be on. Used by `routes/commands.js`, `routes/scaffold.js`, and `routes/git.js` (always scoped) and `routes/detect.js` (scoped only when configured). |
| `workspaceRoots.js` | Shared allow-list for routes that take a caller-supplied filesystem path. `isWithinAllowedRoots(realPath)` is the single complete test — on Windows it also allows any lettered non-system drive (`D:\code`), which appears in no root, so never render `ALLOWED_WORKSPACE_ROOTS` as "the directories you may use". `outsideAllowedRootsMessage(realPath, { field })` formats a redacted server-only diagnostic with the rejected realpath and every checked root. Also `ALLOWED_WORKSPACE_ROOTS` (defaults + `PORTOS_WORKSPACE_ROOTS`, split on the platform path delimiter — `;` on Windows — and symlink-resolved), `isWithinRoot(resolvedPath, root)` (separator-safe containment), and `WORKSPACE_ROOTS_CONFIGURED` (true when the operator set the env var — lets a permissive-by-default route like `routes/detect.js` opt into confinement). Defaults cover home plus wherever the platform mounts secondary volumes: `/tmp` + `/Users` + `/Volumes` + `/mnt` + `/media` + `/opt` on POSIX; home + the temp dir plus the non-system-drive rule on Windows, where those POSIX literals would resolve to whatever drive the process happens to be on. Used by `routes/commands.js`, `routes/scaffold.js`, and `routes/git.js` (always scoped) and `routes/detect.js` (scoped only when configured). |
| `zodCompat.js` | Zod 4 compatibility helpers. `partialWithoutDefaults(objectSchema)` — like `.partial()` but strips inner field defaults first, so a PATCH/update schema doesn't inject (and clobber) the stored values of fields the caller didn't send. Use for any update schema derived from a defaulted base. |

## Test support
Expand Down
92 changes: 91 additions & 1 deletion server/lib/workspaceRoots.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { realpathSync } from 'fs';
import { resolve, relative, isAbsolute, delimiter } from 'path';
import { resolve, relative, isAbsolute, delimiter, basename } from 'path';
import { homedir, tmpdir } from 'os';

// Allowed workspace roots shared by routes that accept a caller-supplied
Expand Down Expand Up @@ -80,3 +80,93 @@ export function isWithinAllowedRoots(realPath) {
if (isOnWindowsWorkspaceDrive(realPath)) return true;
return ALLOWED_WORKSPACE_ROOTS.some(root => isWithinRoot(realPath, root));
}

const HOME_ROOT = ALLOWED_WORKSPACE_ROOTS[0];

const singleLine = (value) => String(value).replace(/[\r\n]+/g, ' ');

const PRIVATE_HOME_SEGMENT = /(^|[\\/])((?:Users|home))([\\/])[^\\/]+(?=([\\/]|$))/gi;

const HOME_USERNAME = basename(homedir());
const HOME_USERNAME_KEY = HOME_USERNAME.toLowerCase();

const redactUncHost = (path) => {
const leading = path.startsWith('//')
? '//'
: path.charCodeAt(0) === 92 && path.charCodeAt(1) === 92
? path.slice(0, 2)
: null;
if (!leading) return path;

const firstSegmentEndOffset = path.slice(leading.length).search(/[\\/]/);
if (firstSegmentEndOffset < 0) return `${leading}<host>`;
const firstSegmentEnd = leading.length + firstSegmentEndOffset;
const firstSegment = path.slice(leading.length, firstSegmentEnd);

// Extended-length UNC paths (`\\?\\UNC\\server\\share`) put the marker and
// `UNC` segments before the actual host. Extended local paths (`\\?\\C:\\…`)
// have no host to redact and fall through to the home-segment redaction.
let hostStart = leading.length;
if (firstSegment === '?' || firstSegment === '.') {
const secondSegmentStart = firstSegmentEnd + 1;
const secondSegmentEndOffset = path.slice(secondSegmentStart).search(/[\\/]/);
if (secondSegmentEndOffset < 0) return path;
const secondSegmentEnd = secondSegmentStart + secondSegmentEndOffset;
const secondSegment = path.slice(secondSegmentStart, secondSegmentEnd);
if (secondSegment.toUpperCase() !== 'UNC') return path;
hostStart = secondSegmentEnd + 1;
}

const hostEndOffset = path.slice(hostStart).search(/[\\/]/);
if (hostEndOffset < 0) return `${path.slice(0, hostStart)}<host>`;
const hostEnd = hostStart + hostEndOffset;
return `${path.slice(0, hostStart)}<host>${path.slice(hostEnd)}`;
};

const redactKnownUsername = (path) => {
if (!HOME_USERNAME) return path;
let result = '';
let segmentStart = 0;
for (let index = 0; index <= path.length; index += 1) {
const atBoundary = index === path.length || path[index] === '/' || path[index] === '\\';
if (!atBoundary) continue;
const segment = path.slice(segmentStart, index);
const matches = IS_WINDOWS
? segment.toLowerCase() === HOME_USERNAME_KEY
: segment === HOME_USERNAME;
result += matches ? '<user>' : segment;
if (index < path.length) result += path[index];
segmentStart = index + 1;
}
return result;
};

const redactPrivatePath = (path) => redactKnownUsername(
path.replace(PRIVATE_HOME_SEGMENT, (_match, prefix, root, separator) => `${prefix}${root}${separator}<user>`)
);

const formatLogPath = (value) => {
const path = singleLine(value);
if (path === HOME_ROOT) return '~';
const redactedUncPath = redactUncHost(path);
const isWindowsStylePath = path.startsWith('//')
|| path.charCodeAt(0) === 92
|| /^[A-Za-z]:[\\/]/.test(path);
if (isWindowsStylePath) return redactPrivatePath(redactedUncPath);
const relativeHomePath = relative(HOME_ROOT, path);
if (relativeHomePath && !relativeHomePath.startsWith('..') && !isAbsolute(relativeHomePath)) {
return `~/${redactPrivatePath(relativeHomePath)}`;
}
return redactPrivatePath(redactedUncPath);
};

/**
* Build the redacted, server-only diagnostic for a path rejected by isWithinAllowedRoots.
* Callers keep their existing terse HTTP error so a real filesystem path never
* appears in an API response or a value a user might paste into a public issue.
*/
export function outsideAllowedRootsMessage(realPath, { field = 'path' } = {}) {
const roots = ALLOWED_WORKSPACE_ROOTS.map(formatLogPath);
if (IS_WINDOWS) roots.push('any non-system drive');
return `${singleLine(field)} is outside allowed directories: ${formatLogPath(realPath)} (allowed: ${roots.join(', ')})`;
}
62 changes: 61 additions & 1 deletion server/lib/workspaceRoots.test.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { describe, it, expect, afterEach, vi } from 'vitest';
import { homedir } from 'os';
import { join, delimiter } from 'path';
import { join, basename, delimiter } from 'path';
import {
isWithinRoot,
isWithinAllowedRoots,
outsideAllowedRootsMessage,
ALLOWED_WORKSPACE_ROOTS,
DEFAULT_WORKSPACE_ROOTS,
} from './workspaceRoots.js';
Expand Down Expand Up @@ -47,6 +48,65 @@ describe('isWithinAllowedRoots', () => {
});
});

describe('outsideAllowedRootsMessage', () => {
it('names the rejected realpath and the roots checked', () => {
const message = outsideAllowedRootsMessage('/etc/shadow');

expect(message).toMatch(/^path is outside allowed directories: \/etc\/shadow \(allowed: .+\)$/);
expect(message).toContain('allowed: ~');
if (IS_WINDOWS) expect(message).toContain('any non-system drive');
});

it('uses a caller-provided field name', () => {
expect(outsideAllowedRootsMessage('/etc/shadow', { field: 'workspacePath' }))
.toMatch(/^workspacePath is outside allowed directories: \/etc\/shadow \(allowed: .+\)$/);
});

it('redacts usernames in other home-directory paths', () => {
const message = outsideAllowedRootsMessage('/home/alice/private-repo');

expect(message).toContain('/home/<user>/private-repo');
expect(message).not.toContain('alice');
});

it('redacts the current username in nonstandard path segments', () => {
const username = basename(homedir());
const message = outsideAllowedRootsMessage(`/srv/${username}/repo`);

expect(message).toContain('<user>');
expect(message).not.toContain(username);
});

it('redacts the current username in nested home-directory segments', () => {
const username = basename(homedir());
const message = outsideAllowedRootsMessage(join(homedir(), 'nested', username, 'repo'));

const expectedNestedPath = IS_WINDOWS
? String.raw`nested\<user>\repo`
: '~/nested/<user>/repo';
expect(message).toContain(expectedNestedPath);
expect(message).not.toContain(username);
});

it('redacts hosts in UNC paths', () => {
const message = outsideAllowedRootsMessage(String.raw`\\server\share\repo`);

expect(message).toContain(String.raw`\\<host>\share\repo`);
expect(message).not.toContain('server');
});

it('redacts nested usernames and extended UNC hosts', () => {
const nestedMessage = outsideAllowedRootsMessage(String.raw`\\server\share\Users\alice\repo`);
const extendedMessage = outsideAllowedRootsMessage(String.raw`\\?\UNC\server\share\repo`);

expect(nestedMessage).toContain(String.raw`\\<host>\share\Users\<user>\repo`);
expect(nestedMessage).not.toContain('server');
expect(nestedMessage).not.toContain('alice');
expect(extendedMessage).toContain(String.raw`\\?\UNC\<host>\share\repo`);
expect(extendedMessage).not.toContain('server');
});
});

// Windows repos routinely live off the system drive (D:\code, E:\projects), and
// the POSIX defaults (/tmp, /Users, /Volumes, /opt) resolve to nothing useful
// there — so a non-system lettered drive is allowed, the way /Volumes is on macOS.
Expand Down
3 changes: 2 additions & 1 deletion server/routes/commands.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { resolve } from 'path';
import * as commands from '../services/commands.js';
import * as pm2Service from '../services/pm2.js';
import { asyncHandler, ServerError } from '../lib/errorHandler.js';
import { isWithinAllowedRoots } from '../lib/workspaceRoots.js';
import { isWithinAllowedRoots, outsideAllowedRootsMessage } from '../lib/workspaceRoots.js';

const router = Router();

Expand Down Expand Up @@ -41,6 +41,7 @@ router.post('/execute', asyncHandler(async (req, res) => {
throw new ServerError('workspacePath is not accessible', { status: 400, code: 'INVALID_PATH' });
}
if (!isWithinAllowedRoots(realPath)) {
console.error(`❌ ${outsideAllowedRootsMessage(realPath, { field: 'workspacePath' })}`);
throw new ServerError('workspacePath is outside allowed directories', { status: 400, code: 'INVALID_PATH' });
}
}
Expand Down
3 changes: 2 additions & 1 deletion server/routes/detect.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { execPm2 } from '../services/pm2.js';
import { detectAppWithAi } from '../services/aiDetect.js';
import { asyncHandler, ServerError } from '../lib/errorHandler.js';
import { safeJSONParse, tryReadFile } from '../lib/fileUtils.js';
import { isWithinAllowedRoots, WORKSPACE_ROOTS_CONFIGURED } from '../lib/workspaceRoots.js';
import { isWithinAllowedRoots, outsideAllowedRootsMessage, WORKSPACE_ROOTS_CONFIGURED } from '../lib/workspaceRoots.js';

const execAsync = promisify(exec);
const router = Router();
Expand Down Expand Up @@ -68,6 +68,7 @@ router.post('/repo', asyncHandler(async (req, res) => {
return res.json({ valid: false, error: 'Path is not accessible' });
}
if (!isWithinAllowedRoots(realPath)) {
console.error(`❌ ${outsideAllowedRootsMessage(realPath)}`);
return res.json({
valid: false,
error: 'Path is outside the configured workspace roots (PORTOS_WORKSPACE_ROOTS)'
Expand Down
3 changes: 2 additions & 1 deletion server/routes/git.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import * as git from '../services/git.js';
import * as appsService from '../services/apps.js';
import { getAgents } from '../services/cosAgentLifecycle.js';
import { asyncHandler, ServerError } from '../lib/errorHandler.js';
import { isWithinAllowedRoots } from '../lib/workspaceRoots.js';
import { isWithinAllowedRoots, outsideAllowedRootsMessage } from '../lib/workspaceRoots.js';
import { validateRequest, submoduleStatusQuerySchema, submoduleUpdateSchema } from '../lib/validation.js';

/**
Expand All @@ -32,6 +32,7 @@ function assertAllowedWorkspace(path) {
throw new ServerError('path is not accessible', { status: 400, code: 'INVALID_PATH' });
}
if (!isWithinAllowedRoots(realPath)) {
console.error(`❌ ${outsideAllowedRootsMessage(realPath)}`);
throw new ServerError('path is outside allowed directories', { status: 403, code: 'FORBIDDEN' });
}
}
Expand Down
6 changes: 4 additions & 2 deletions server/routes/git.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ vi.mock('../services/cosAgentLifecycle.js', () => ({
// Mock workspace-roots so we control which paths are "allowed" without
// touching the real filesystem.
vi.mock('../lib/workspaceRoots.js', () => ({
isWithinAllowedRoots: vi.fn()
isWithinAllowedRoots: vi.fn(),
outsideAllowedRootsMessage: vi.fn((realPath, { field = 'path' } = {}) => `${field} is outside allowed directories: ${realPath}`)
}));

// Mock fs functions used by assertAllowedWorkspace.
Expand All @@ -59,7 +60,7 @@ vi.mock('fs', async (importOriginal) => {
});

import { existsSync, statSync, realpathSync } from 'fs';
import { isWithinAllowedRoots } from '../lib/workspaceRoots.js';
import { isWithinAllowedRoots, outsideAllowedRootsMessage } from '../lib/workspaceRoots.js';
import * as cosAgentLifecycleService from '../services/cosAgentLifecycle.js';
import * as gitService from '../services/git.js';

Expand Down Expand Up @@ -99,6 +100,7 @@ describe('git routes — workspace root validation', () => {

expect(res.status).toBe(403);
expect(res.body.code).toBe('FORBIDDEN');
expect(outsideAllowedRootsMessage).toHaveBeenCalledWith(body.path);
});
});

Expand Down
3 changes: 2 additions & 1 deletion server/routes/scaffold.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { createApp, getReservedPorts } from '../services/apps.js';
import { asyncHandler, ServerError } from '../lib/errorHandler.js';
import { validateRequest, scaffoldSchema } from '../lib/validation.js';
import { ensureDir, expandHome } from '../lib/fileUtils.js';
import { isWithinAllowedRoots } from '../lib/workspaceRoots.js';
import { isWithinAllowedRoots, outsideAllowedRootsMessage } from '../lib/workspaceRoots.js';
import { scaffoldVite } from './scaffoldVite.js';
import { scaffoldExpress } from './scaffoldExpress.js';
import { scaffoldIOS } from './scaffoldIOS.js';
Expand Down Expand Up @@ -259,6 +259,7 @@ async function scaffoldApp(req, res) {
throw new ServerError('parentDir is not accessible', { status: 400, code: 'INVALID_PARENT' });
}
if (!isWithinAllowedRoots(realParentDir)) {
console.error(`❌ ${outsideAllowedRootsMessage(realParentDir, { field: 'parentDir' })}`);
throw new ServerError('parentDir is outside allowed directories', { status: 403, code: 'FORBIDDEN' });
}

Expand Down
3 changes: 2 additions & 1 deletion server/routes/scaffold.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ vi.mock('../services/apps.js', () => ({
}));

vi.mock('../lib/workspaceRoots.js', () => ({
isWithinAllowedRoots: vi.fn(() => true)
isWithinAllowedRoots: vi.fn(() => true),
outsideAllowedRootsMessage: vi.fn((realPath, { field = 'path' } = {}) => `${field} is outside allowed directories: ${realPath}`)
}));

vi.mock('./scaffoldVite.js', () => ({ scaffoldVite: vi.fn().mockResolvedValue(undefined) }));
Expand Down