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 src/__tests__/dashboard-session-auth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ describe('dashboard session cookie request auth', () => {
url: '/v1/owned/other-tenant',
headers: { cookie: cookieHeader(dashboardSession.sessionId) },
});
expect(otherTenant.statusCode).toBe(403);
expect(otherTenant.statusCode).toBe(404);
});

it('filters global SSE events for tenant-scoped dashboard sessions', () => {
Expand Down
2 changes: 1 addition & 1 deletion src/__tests__/multitenancy-1944.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ describe('Multi-tenancy (#1944) — requireOwnership with tenant', () => {
const result = requireOwnership(sessions, 's-1', reply, 'globex-key', 'globex');

expect(result).toBeNull();
expect(reply.status).toHaveBeenCalledWith(403);
expect(reply.status).toHaveBeenCalledWith(404);
});

it('allows same-tenant access', async () => {
Expand Down
3 changes: 2 additions & 1 deletion src/permission-routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ function createPermissionHandler(
if (callerTenantId && callerTenantId !== SYSTEM_TENANT && session.tenantId !== callerTenantId) {
const effectiveAudit = getAuditLogger ? getAuditLogger() : audit;
if (effectiveAudit) void effectiveAudit.log(keyId ?? 'system', 'session.action.denied', `Cross-tenant ${matchedPermission} denied on session ${session.id} (tenant: ${session.tenantId})`, session.id, callerTenantId);
return reply.status(403).send({ error: 'SESSION_FORBIDDEN', message: 'Session belongs to another tenant' });
// #3360: Return generic 404 to avoid leaking cross-tenant session existence.
return reply.status(404).send({ error: 'SESSION_NOT_FOUND', message: 'Session not found' });
}

// Feature flag check (#1910): skip enhanced ownership when disabled
Expand Down
3 changes: 2 additions & 1 deletion src/routes/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,8 +221,9 @@ export function requireOwnership(
if (keyId === 'master' || keyId === null || keyId === undefined) return session;
// Issue #2267: Tenant scoping — reject cross-tenant access.
// SYSTEM_TENANT callers bypass scoping. Tenant-scoped callers can only access their own sessions.
// #3360: Return generic 404 to avoid leaking that a cross-tenant session exists.
if (tenantId && tenantId !== SYSTEM_TENANT && session.tenantId !== tenantId) {
reply.status(403).send({ error: 'Forbidden: session belongs to another tenant' });
reply.status(404).send({ error: 'Session not found' });
return null;
}
if (role === 'admin') return session;
Expand Down
6 changes: 3 additions & 3 deletions src/services/acp/local-storage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,16 +168,17 @@ export class FileAcpLocalStorageProfile implements AcpLocalStorageProfile {
}
// Issue #3045: atomic write to prevent truncation on SIGTERM/OOM kill
// Issue #3366: error recovery — a failed write must not poison subsequent writes
// #3363: restrict file permissions to owner-only
const content = `${JSON.stringify(serializeState(this.state), null, 2)}\n`;
const tmpFile = `${this.config.filePath}.tmp.${process.pid}`;

const prevChain = this.writeChain;
this.writeChain = prevChain
.then(
// Previous write succeeded — do this write
() => writeFile(tmpFile, content, 'utf8').then(() => rename(tmpFile, this.config.filePath)),
() => writeFile(tmpFile, content, { mode: 0o600 }).then(() => rename(tmpFile, this.config.filePath)),
// Previous write failed — still attempt this write
() => writeFile(tmpFile, content, 'utf8').then(() => rename(tmpFile, this.config.filePath)),
() => writeFile(tmpFile, content, { mode: 0o600 }).then(() => rename(tmpFile, this.config.filePath)),
)
.then(() => {
this.persistError = null;
Expand All @@ -195,7 +196,6 @@ export class FileAcpLocalStorageProfile implements AcpLocalStorageProfile {
// Reset chain so next persist() is not chained to a rejected promise
this.writeChain = Promise.resolve();
});

await this.writeChain;
}

Expand Down
2 changes: 1 addition & 1 deletion src/services/metrics-cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ export class JsonFileBackend implements MetricsCacheBackend {
await mkdir(dir, { recursive: true });
}
const tmpFile = `${this.filePath}.tmp`;
await writeFile(tmpFile, JSON.stringify(data, null, 2));
await writeFile(tmpFile, JSON.stringify(data, null, 2), { mode: 0o600 });
await rename(tmpFile, this.filePath);
}
}
Expand Down
9 changes: 5 additions & 4 deletions src/services/state/JsonFileStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
* Issue #1937: Pluggable SessionStore interface.
*/

import { readFile, writeFile, rename, mkdir } from 'node:fs/promises';
import { readFile, writeFile, rename, mkdir, chmod } from 'node:fs/promises';
import { existsSync, unlinkSync, readdirSync } from 'node:fs';
import { join, dirname } from 'node:path';
import { Mutex } from 'async-mutex';
Expand Down Expand Up @@ -85,7 +85,7 @@ export class JsonFileStore implements StateStore {
if (this.isValidState(parsed)) {
// Write backup of successfully loaded state
try {
await writeFile(`${this.stateFile}.bak`, raw);
await writeFile(`${this.stateFile}.bak`, raw, { mode: 0o600 });
} catch { /* non-critical */ }
return parsed as SerializedSessionState;
}
Expand All @@ -112,12 +112,13 @@ export class JsonFileStore implements StateStore {
async save(state: SerializedSessionState): Promise<void> {
// #2793: Use unique temp file per save to prevent ENOENT race when
// concurrent saves (SessionManager.doSave + putSession) overlap.
// #3363: State files contain sensitive metadata — restrict to owner-only.
const dir = dirname(this.stateFile);
if (!existsSync(dir)) {
await mkdir(dir, { recursive: true });
}
const tmpFile = `${this.stateFile}.tmp.${process.pid}.${Date.now()}.${Math.random().toString(36).slice(2)}`;
await writeFile(tmpFile, JSON.stringify(state, null, 2));
await writeFile(tmpFile, JSON.stringify(state, null, 2), { mode: 0o600 });
await rename(tmpFile, this.stateFile);
}

Expand Down Expand Up @@ -199,7 +200,7 @@ export class JsonFileStore implements StateStore {
}

const tmpFile = `${this.pipelineFile}.tmp.${process.pid}.${Date.now()}.${Math.random().toString(36).slice(2)}`;
await writeFile(tmpFile, JSON.stringify(state, null, 2));
await writeFile(tmpFile, JSON.stringify(state, null, 2), { mode: 0o600 });
await rename(tmpFile, this.pipelineFile);
}

Expand Down
10 changes: 2 additions & 8 deletions src/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -744,10 +744,7 @@ export async function validateWorkDir(
const resolved = path.resolve(workDir);
const preAllowed = candidateSafeDirs.some((dir) => isUnderOrEqual(resolved, dir));
if (!preAllowed) {
const allowedList = candidateSafeDirs.length <= 5
? candidateSafeDirs.join(', ')
: candidateSafeDirs.slice(0, 5).join(', ') + ` (+${candidateSafeDirs.length - 5} more)`;
return { error: `workDir ${resolved} is not in the allowed directories list. Allowed: ${allowedList}`, code: 'INVALID_WORKDIR' };
return { error: `workDir ${resolved} is not in the allowed directories list`, code: 'INVALID_WORKDIR' };
}

// Step 3: Follow symlinks.
Expand All @@ -761,10 +758,7 @@ export async function validateWorkDir(
// Step 4: Canonical allowlist check after symlink resolution.
const allowed = candidateSafeDirs.some((dir) => isUnderOrEqual(realPath, dir));
if (!allowed) {
const allowedList = candidateSafeDirs.length <= 5
? candidateSafeDirs.join(', ')
: candidateSafeDirs.slice(0, 5).join(', ') + ` (+${candidateSafeDirs.length - 5} more)`;
return { error: `workDir ${resolved} is not in the allowed directories list. Allowed: ${allowedList}`, code: 'INVALID_WORKDIR' };
return { error: `workDir ${resolved} is not in the allowed directories list`, code: 'INVALID_WORKDIR' };
}

// Step 5: Ensure workDir is a directory, not a file.
Expand Down
Loading