tighten permission checks and input validation across the api

tests/backups/security.test.ts

@@ -63,6 +63,40 @@ describe('isPathWithin (restore-target containment)', () => {
 	});
 });
 
+describe('unsafeRestoreTargetReason: /usr and .ssh takeover targets', () => {
+	it('blocks anywhere under /usr (PATH hijack, systemd units) - it holds no user data', () => {
+		for (const p of [
+			'/usr/local/bin/docker-entrypoint',
+			'/usr/lib/systemd/system/evil.service',
+			'/usr/local/share/app',
+			'/usr',
+		]) {
+			expect(unsafeRestoreTargetReason(p)).not.toBeNull();
+		}
+	});
+
+	it('blocks any .ssh directory segment (SSH-key takeover under any home)', () => {
+		for (const p of [
+			'/home/user/.ssh/authorized_keys',
+			'/root/.ssh/authorized_keys',
+			'/data/.ssh/keys',
+		]) {
+			expect(unsafeRestoreTargetReason(p)).not.toBeNull();
+		}
+	});
+
+	it('still allows a user-home data path and an .ssh-lookalike segment', () => {
+		expect(unsafeRestoreTargetReason('/home/user/backup')).toBeNull();
+		expect(unsafeRestoreTargetReason('/home/user/data/.sshconfig')).toBeNull();
+		expect(unsafeRestoreTargetReason('/mnt/.ssh-backups/x')).toBeNull();
+	});
+
+	it('boundary-safe: a sibling that merely shares a forbidden prefix is allowed', () => {
+		expect(unsafeRestoreTargetReason('/usr-data/x')).toBeNull();
+		expect(unsafeRestoreTargetReason('/home2/restore')).toBeNull();
+	});
+});
+
 describe('isSafePathSegment (name traversal guard)', () => {
 	it('accepts a plain name', () => {
 		expect(isSafePathSegment('web')).toBe(true);
@@ -141,7 +175,8 @@ describe('re-exported hardened primitives still reject dangerous inputs', () =>
 
 describe('unsafeRestoreTargetReason (new-location targetPath containment)', () => {
 	it('accepts legitimate operator-chosen restore locations', () => {
-		for (const p of ['/mnt/restore', '/srv/appA/data', '/home/user/backup', '/opt/data', '/data/x', '/usr/local/share/app']) {
+		// /usr/local/share is NOT here: /usr is blocked wholesale (host-takeover surface).
+		for (const p of ['/mnt/restore', '/srv/appA/data', '/home/user/backup', '/opt/data', '/data/x']) {
 			expect(unsafeRestoreTargetReason(p)).toBeNull();
 		}
 	});

tests/fs-guard.test.ts

@@ -1,4 +1,7 @@
-import { describe, test, expect, beforeAll } from 'bun:test';
+import { describe, test, expect, beforeAll, afterAll } from 'bun:test';
+import { mkdtempSync, mkdirSync, symlinkSync, writeFileSync, rmSync } from 'node:fs';
+import { join } from 'node:path';
+import { tmpdir } from 'node:os';
 
 // Pin DATA_DIR before importing so the guard computes /app/data paths.
 beforeAll(() => { process.env.DATA_DIR = '/app/data'; });
@@ -24,17 +27,44 @@ describe('isProtectedPath (file-browser secret guard, H1)', () => {
 		expect(isProtectedPath('/proc/self/cmdline')).toBe(true);
 	});
 
-	test('does NOT block the stacks dir or external stacks (browsing preserved)', () => {
+	test('blocks system secret trees /etc and /root', () => {
+		expect(isProtectedPath('/etc')).toBe(true);
+		expect(isProtectedPath('/etc/shadow')).toBe(true);
+		expect(isProtectedPath('/etc/passwd')).toBe(true);
+		expect(isProtectedPath('/etc/hostname')).toBe(true);
+		expect(isProtectedPath('/root')).toBe(true);
+		expect(isProtectedPath('/root/.bashrc')).toBe(true);
+	});
+
+	test('blocks a .git dir inside a git-repos clone (config embeds credentials)', () => {
+		expect(isProtectedPath('/app/data/git-repos/some-repo/.git')).toBe(true);
+		expect(isProtectedPath('/app/data/git-repos/some-repo/.git/config')).toBe(true);
+		expect(isProtectedPath('/any/path/.git/config')).toBe(true);
+	});
+
+	test('blocks any .ssh path segment at any depth', () => {
+		expect(isProtectedPath('/home/user/.ssh')).toBe(true);
+		expect(isProtectedPath('/home/user/.ssh/id_rsa')).toBe(true);
+		expect(isProtectedPath('/some/deep/nested/.ssh/known_hosts')).toBe(true);
+	});
+
+	test('does NOT block the stacks dir, external stacks, or git-repos working copies (browsing preserved)', () => {
 		expect(isProtectedPath('/app/data')).toBe(false);
 		expect(isProtectedPath('/app/data/stacks')).toBe(false);
 		expect(isProtectedPath('/app/data/stacks/myapp/docker-compose.yml')).toBe(false);
 		expect(isProtectedPath('/external-stacks')).toBe(false);
 		expect(isProtectedPath('/external-stacks/app/.env')).toBe(false);
+		// The git-repos dir and its working copies stay browsable (deploy verification,
+		// adoption); only the .git subdir inside a clone is hidden.
+		expect(isProtectedPath('/app/data/git-repos')).toBe(false);
+		expect(isProtectedPath('/app/data/git-repos/prod/some-repo')).toBe(false);
+		expect(isProtectedPath('/app/data/git-repos/prod/some-repo/docker-compose.yml')).toBe(false);
 	});
 
 	test('does NOT block unrelated container paths', () => {
-		expect(isProtectedPath('/etc/hostname')).toBe(false);
 		expect(isProtectedPath('/tmp/x')).toBe(false);
+		expect(isProtectedPath('/opt/stacks/app/compose.yaml')).toBe(false);
+		expect(isProtectedPath('/home/user/docker/app/.env')).toBe(false);
 	});
 
 	test('is a path-boundary check, not a string prefix (no false positives)', () => {
@@ -43,5 +73,57 @@ describe('isProtectedPath (file-browser secret guard, H1)', () => {
 		expect(isProtectedPath('/app/data/database-backups')).toBe(false);
 		// a file merely named like the key elsewhere is fine
 		expect(isProtectedPath('/app/data/stacks/.encryption_key')).toBe(false);
+		// "/etcd" / "/rootfs" are NOT inside "/etc" / "/root"
+		expect(isProtectedPath('/etcd/data')).toBe(false);
+		expect(isProtectedPath('/rootfs/app')).toBe(false);
+		// ".sshfoo" / ".github" are different segment names, not .ssh / .git
+		expect(isProtectedPath('/home/user/.sshfoo/x')).toBe(false);
+		expect(isProtectedPath('/app/data/stacks/myapp/.github/workflows')).toBe(false);
+	});
+});
+
+describe('isProtectedPath symlink resolution (the reason safeResolve exists)', () => {
+	let scratch: string;
+
+	beforeAll(() => {
+		scratch = mkdtempSync(join(tmpdir(), 'fsguard-'));
+		// A protected system tree reached via a symlink placed in an allowed dir.
+		mkdirSync(join(scratch, 'allowed'), { recursive: true });
+		symlinkSync('/etc', join(scratch, 'allowed', 'sys-link'));
+		// A benign control symlink to a non-protected dir.
+		mkdirSync(join(scratch, 'benign-target'), { recursive: true });
+		symlinkSync(join(scratch, 'benign-target'), join(scratch, 'allowed', 'ok-link'));
+	});
+
+	afterAll(() => { rmSync(scratch, { recursive: true, force: true }); });
+
+	test('a symlink pointing into a protected root is blocked (not just literal /etc)', () => {
+		// resolve() alone would leave the unprotected scratch path and pass; only
+		// safeResolve following the link to /etc catches this.
+		expect(isProtectedPath(join(scratch, 'allowed', 'sys-link'))).toBe(true);
+		expect(isProtectedPath(join(scratch, 'allowed', 'sys-link', 'shadow'))).toBe(true);
+	});
+
+	test('a symlink to a non-protected dir stays allowed (no over-blocking)', () => {
+		expect(isProtectedPath(join(scratch, 'allowed', 'ok-link'))).toBe(false);
+		expect(isProtectedPath(join(scratch, 'allowed', 'ok-link', 'compose.yaml'))).toBe(false);
+	});
+
+	test('a symlinked encryption-key / db target is still blocked (resolved on both sides)', () => {
+		// Simulate DATA_DIR/.encryption_key mounted from a secret store: the guard
+		// must resolve the TARGET too, or the request (resolved to the target) never
+		// matches the unresolved keyFile constant.
+		const dataDir = mkdtempSync(join(tmpdir(), 'fsguard-data-'));
+		mkdirSync(join(dataDir, 'real-secrets'), { recursive: true });
+		writeFileSync(join(dataDir, 'real-secrets', 'key'), 'master');
+		symlinkSync(join(dataDir, 'real-secrets', 'key'), join(dataDir, '.encryption_key'));
+		const prev = process.env.DATA_DIR;
+		process.env.DATA_DIR = dataDir;
+		try {
+			expect(isProtectedPath(join(dataDir, '.encryption_key'))).toBe(true);
+		} finally {
+			process.env.DATA_DIR = prev;
+			rmSync(dataDir, { recursive: true, force: true });
+		}
 	});
 });

tests/provider-form-config.test.ts

@@ -1,6 +1,6 @@
 import { describe, test, expect } from 'bun:test';
 import { collectProviderFormConfig } from '../src/lib/utils/provider-form-config';
-import { mergeProviderConfigForWrite } from '../src/lib/server/secretproviders/shared';
+import { mergeProviderConfigForWrite, destinationOverridesStored } from '../src/lib/server/secretproviders/shared';
 
 // Field shape (key + isSecret) per provider, mirroring PROVIDER_FIELDS in
 // routes/settings/secrets/ProviderModal.svelte. Only the shape matters for what the edit
@@ -181,3 +181,43 @@ describe('#1448 full chain: form output -> server merge', () => {
 		expect(merged.mount).toBe(''); // server reads '' as the default mount
 	});
 });
+
+// The test endpoint must not send a stored credential to a caller-chosen host. It decides via
+// destinationOverridesStored: if the override changes host/address, the stored secret is NOT
+// reattached and the caller needs secrets:edit.
+describe('destinationOverridesStored (credential-exfil guard)', () => {
+	test('Vault: changed address is a redirect', () => {
+		const stored = { address: 'https://vault.internal:8200', token: 'hvs.REAL' };
+		expect(destinationOverridesStored({ address: 'http://attacker.example:8902' }, stored)).toBe(true);
+	});
+
+	test('same address (edit form keeps host, blank token) is NOT a redirect', () => {
+		const stored = { address: 'https://vault.internal:8200', token: 'hvs.REAL' };
+		expect(destinationOverridesStored({ address: 'https://vault.internal:8200', mount: 'kv2' }, stored)).toBe(false);
+	});
+
+	test('1Password Connect / Infisical: changed host is a redirect', () => {
+		const stored = { host: 'https://connect.internal', token: 'ey.REAL' };
+		expect(destinationOverridesStored({ host: 'https://evil.example' }, stored)).toBe(true);
+	});
+
+	test('Bitwarden: changed serverUrl is a redirect (full stored-token leak vector)', () => {
+		const stored = { serverUrl: 'https://vault.bitwarden.eu', token: 'bws.REAL' };
+		expect(destinationOverridesStored({ serverUrl: 'http://attacker.example:8902' }, stored)).toBe(true);
+	});
+
+	test('Azure Key Vault: changed vaultUri is a redirect', () => {
+		const stored = { vaultUri: 'https://kv.vault.azure.net', clientSecret: 'REAL' };
+		expect(destinationOverridesStored({ vaultUri: 'https://evil.example' }, stored)).toBe(true);
+	});
+
+	test('override that omits the destination entirely is NOT a redirect', () => {
+		const stored = { address: 'https://vault.internal:8200', token: 'hvs.REAL' };
+		expect(destinationOverridesStored({ mount: 'kv2' }, stored)).toBe(false);
+	});
+
+	test('a cleared (blank) destination is not treated as a redirect', () => {
+		const stored = { host: 'https://connect.internal', token: 'ey.REAL' };
+		expect(destinationOverridesStored({ host: '   ' }, stored)).toBe(false);
+	});
+});

tests/secret-redact.test.ts

@@ -0,0 +1,63 @@
+import { describe, test, expect } from 'bun:test';
+import { redactSecretValues, redactSecretVars } from '../src/lib/server/secret-redact';
+
+describe('redactSecretValues', () => {
+	test('replaces a secret value with [REDACTED]', () => {
+		const out = redactSecretValues('exec: "s3cr3t-pw": not found', ['s3cr3t-pw']);
+		expect(out).toBe('exec: "[REDACTED]": not found');
+		expect(out).not.toContain('s3cr3t-pw');
+	});
+
+	test('replaces every occurrence', () => {
+		const out = redactSecretValues('pw=hunter2 again hunter2', ['hunter2']);
+		expect(out).toBe('pw=[REDACTED] again [REDACTED]');
+	});
+
+	test('redacts multiple distinct values', () => {
+		const out = redactSecretValues('a=alpha11 b=bravo22', ['alpha11', 'bravo22']);
+		expect(out).toBe('a=[REDACTED] b=[REDACTED]');
+	});
+
+	test('masks the longest value fully when one contains another', () => {
+		// 'superpassword' contains 'password'; longest-first prevents a partial leak.
+		const out = redactSecretValues('x=superpassword', ['password', 'superpassword']);
+		expect(out).toBe('x=[REDACTED]');
+		expect(out).not.toContain('password');
+	});
+
+	test('skips values shorter than 4 chars (would corrupt ordinary output)', () => {
+		const out = redactSecretValues('exit code 1, port 22', ['1', '22']);
+		expect(out).toBe('exit code 1, port 22');
+	});
+
+	test('returns text unchanged when no values match', () => {
+		expect(redactSecretValues('nothing secret here', ['absent-value'])).toBe('nothing secret here');
+	});
+
+	test('handles empty text and empty values', () => {
+		expect(redactSecretValues('', ['secret'])).toBe('');
+		expect(redactSecretValues('text', [])).toBe('text');
+	});
+
+	test('dedupes repeated values without error', () => {
+		const out = redactSecretValues('v=mysecretv', ['mysecretv', 'mysecretv']);
+		expect(out).toBe('v=[REDACTED]');
+	});
+});
+
+describe('redactSecretVars', () => {
+	test('redacts the values of a secret-vars map', () => {
+		const out = redactSecretVars('DB error: pass=topsecret123', { DB_PASSWORD: 'topsecret123' });
+		expect(out).toBe('DB error: pass=[REDACTED]');
+	});
+
+	test('undefined map returns text unchanged', () => {
+		expect(redactSecretVars('anything', undefined)).toBe('anything');
+	});
+
+	test('only the VALUE is redacted, not the key name', () => {
+		const out = redactSecretVars('DB_PASSWORD not set: topsecret123', { DB_PASSWORD: 'topsecret123' });
+		expect(out).toContain('DB_PASSWORD');
+		expect(out).not.toContain('topsecret123');
+	});
+});

tests/stack-delete-guard.test.ts

@@ -43,4 +43,14 @@ describe('isDeletableStackDir', () => {
 	it('a "../" escape resolves out and is NOT deletable', () => {
 		expect(isDeletableStackDir('/data/stacks/../evil', STACKS, 'evil')).toBe(false);
 	});
+
+	it('traversal name "..": getStackDir("..") resolves to DATA_DIR and is NOT deletable', () => {
+		// join(STACKS, '..') = '/data' (DATA_DIR). basename '/data' = 'data' != '..' AND not a
+		// strict subdir of /data/stacks -> blocked. Guards the removeStack fallback branch.
+		expect(isDeletableStackDir('/data', STACKS, '..')).toBe(false);
+	});
+
+	it('traversal name "../..": resolves above DATA_DIR and is NOT deletable', () => {
+		expect(isDeletableStackDir('/', STACKS, '../..')).toBe(false);
+	});
 });

tests/unit/webhook-secret-policy.test.ts

@@ -1,6 +1,6 @@
 // @ts-expect-error -- bun:test is a runtime built-in with no types installed
 import { describe, test, expect, afterEach } from 'bun:test';
-import { decideWebhookSecretPolicy, allowSecretlessWebhook } from '../../src/lib/server/webhook-secret-policy';
+import { decideWebhookSecretPolicy, allowSecretlessWebhook, webhookConfigRequiresSecret } from '../../src/lib/server/webhook-secret-policy';
 
 // This proves the pure policy (all 4 combinations) + the env-flag parsing. The webhook
 // handler is a thin wiring of decideWebhookSecretPolicy; the deploy-unverified branch
@@ -26,6 +26,23 @@ describe('decideWebhookSecretPolicy', () => {
 	});
 });
 
+describe('webhookConfigRequiresSecret (create/update gate)', () => {
+	test('enabled + no secret + no opt-out -> must reject (secure default)', () => {
+		expect(webhookConfigRequiresSecret(true, false, false)).toBe(true);
+	});
+	test('enabled + no secret + opt-out on -> allowed (the escape hatch that was unreachable)', () => {
+		expect(webhookConfigRequiresSecret(true, false, true)).toBe(false);
+	});
+	test('enabled + secret present -> allowed regardless of opt-out', () => {
+		expect(webhookConfigRequiresSecret(true, true, false)).toBe(false);
+		expect(webhookConfigRequiresSecret(true, true, true)).toBe(false);
+	});
+	test('webhook disabled -> never requires a secret', () => {
+		expect(webhookConfigRequiresSecret(false, false, false)).toBe(false);
+		expect(webhookConfigRequiresSecret(false, false, true)).toBe(false);
+	});
+});
+
 describe('allowSecretlessWebhook (env flag)', () => {
 	const original = process.env.ALLOW_WEBHOOKS_WITHOUT_SECRET;
 	afterEach(() => {