fix(remote): replace explicit Windows ACL grants with current-user access

This commit is contained in:
ruv committed 2026-10-12 11:56:13 -04:00
1 parent 2c1222789d
commit 720cfd7c8a
4 files changed
+61 -12

No files matched your search

+2 -2
View File
@@ -66,7 +66,7 @@
"src/policy.js": "baad5eb8c3eb2e416e0a71b342ad850d7c8413d7d976571ebf596937f69da8e2",
"src/process-runner.js": "49533b038044dfb8bc76ed01c030d06a9856ead0836157fb693e2a7d40f786d6",
"src/redact.js": "ebf1afff46341078706b0401838c53db043603586e280d51ece5cf1feba35189",
"src/remote.js": "ca510804176205eca799b396c0f0bca72df4c08139547c66483885d38fed6815",
"src/remote.js": "68f901411063fd515a52dad41ffd5c599765a53de34a080715232825eaf207b3",
"src/repo-trust.js": "06e2a94d7113ed936f208a12b7fcc785801c215a3e2c5e7418f6238d991a289c",
"src/sdk.d.ts": "d4c80689248c197b2ddb1ebf4cd7d8f2e55716722074d9e1cdb8029b2dfa19f7",
"src/sdk.js": "1ac29fb389ed86e0f62ecfddbbd31614c58e832a8181495caa83fe5cf8b30bc4",
@@ -75,7 +75,7 @@
"src/training.js": "bd8a6c12eba4f9d2f8990fa9692cd04c743ea247f9b13e10f8717ceefcbd09ac",
"src/ui/console-widget.js": "0d2f707d57b5ec54968dee1dc80f31c7db9152dcadfc48700c18379dabb55462"
},
"filesDigest": "52d75ef1509cb993b96291fd28836baeb284a05a99ae1e7fac6a1356f9d80b7a",
"filesDigest": "f609508982d1fdc67fa7b90e1286e25945f920ba45e8fde95f76234848cf3be9",
"brainDigest": "c0fb7b079ded157059b91601361429944697dae3cc42abc00dfe1a680986b0f4",
"gateFingerprint": "6e53c784eee38310188948fc75fb49e6b4ebc04e247d01b903fa8c8a92d67bdd",
"developmentPins": {
+1 -1
View File
@@ -1 +1 @@
c76cd7d8c4e204d8b8ba2fa9fdc787fcaa6adcea7fb6f0c62328ce87d01ab390 manifest.json
4c69daba76af8460fe5101da07aeab58cd84376ca642280e038f52cf3c4676d6 manifest.json
+25 -4
View File
@@ -9,7 +9,7 @@
import { spawnSync } from 'node:child_process';
import { existsSync, mkdirSync, readFileSync, writeFileSync } from 'node:fs';
import { homedir, userInfo } from 'node:os';
import { homedir } from 'node:os';
import { dirname, join } from 'node:path';
const NAME_RE = /^[a-z0-9][a-z0-9-]{0,31}$/;
@@ -36,16 +36,37 @@ export function loadHosts(path = hostsFile()) {
return { path, hosts: (data.hosts || []).map(validateHost) };
}
// A fixed program receives the path as JSON on stdin, never PowerShell source.
// Construct a fresh protected DACL: icacls /inheritance:r /grant:r leaves other
// explicit grants intact (including grants on hosted Windows temporary files).
const OWNER_ONLY_ACL = [
"$ErrorActionPreference = 'Stop'",
'[Console]::InputEncoding = [System.Text.UTF8Encoding]::new($false)',
'$request = [Console]::In.ReadToEnd() | ConvertFrom-Json',
'$sid = [System.Security.Principal.WindowsIdentity]::GetCurrent().User',
'$acl = [System.Security.AccessControl.FileSecurity]::new()',
'$acl.SetOwner($sid)',
'$acl.SetAccessRuleProtection($true, $false)',
'$rule = [System.Security.AccessControl.FileSystemAccessRule]::new($sid, [System.Security.AccessControl.FileSystemRights]::FullControl, [System.Security.AccessControl.AccessControlType]::Allow)',
'$acl.AddAccessRule($rule)',
'[System.IO.File]::SetAccessControl([string]$request.path, $acl)',
'$applied = Get-Acl -LiteralPath $request.path',
'$rules = @($applied.GetAccessRules($true, $true, [System.Security.Principal.SecurityIdentifier]))',
'if (-not $applied.AreAccessRulesProtected -or $rules.Count -ne 1 -or $rules[0].IdentityReference.Value -ne $sid.Value -or $rules[0].AccessControlType -ne [System.Security.AccessControl.AccessControlType]::Allow -or $rules[0].FileSystemRights -ne [System.Security.AccessControl.FileSystemRights]::FullControl) { throw "owner-only ACL verification failed" }',
].join('\n');
/**
* Windows ignores POSIX modes, so 0o600 alone leaves the file readable through
* inherited ACLs. Replace them with a single full-control grant for the owner.
* Fails closed: an unrestricted hosts file is an error, not a warning.
*/
export function restrictToOwner(path, { platform = process.platform, exec = spawnSync, user = () => userInfo().username } = {}) {
export function restrictToOwner(path, { platform = process.platform, exec = spawnSync } = {}) {
if (platform !== 'win32') return 'posix-0600';
const r = exec('icacls', [path, '/inheritance:r', '/grant:r', `${user()}:F`], { encoding: 'utf8', windowsHide: true });
const r = exec('powershell.exe', ['-NoLogo', '-NoProfile', '-NonInteractive', '-Command', OWNER_ONLY_ACL], {
input: JSON.stringify({ path }), encoding: 'utf8', windowsHide: true, timeout: 10_000, maxBuffer: 16_384,
});
if (r.error || r.status !== 0) {
throw new Error(`could not restrict ${path} to the current user (icacls exit ${r.status ?? r.error?.code}); the hosts file was not made private`);
throw new Error(`could not restrict ${path} to the current user (ACL helper exit ${r.status ?? r.error?.code}); the hosts file was not made private`);
}
return 'windows-acl-owner-only';
}
+33 -5
View File
@@ -2,7 +2,7 @@ import test from 'node:test';
import assert from 'node:assert/strict';
import { existsSync, mkdtempSync, readFileSync, rmSync, statSync } from 'node:fs';
import { spawnSync } from 'node:child_process';
import { tmpdir } from 'node:os';
import { tmpdir, userInfo } from 'node:os';
import { dirname, join, resolve } from 'node:path';
import { buildSshArgs, restrictToOwner, runRemote, saveHost, shellQuote, validateHost } from '../src/remote.js';
import { isRemoteReadOnlyTool, runTool, TOOLS } from '../src/tools.js';
@@ -64,6 +64,7 @@ test('hosts file is private and remote runs are read-only, validated, and versio
const grants = acl.split('\n').map((l) => l.replace(path, '').trim()).filter((l) => /:\(/.test(l));
assert.equal(grants.length, 1, acl);
assert.match(grants[0], /:\(F\)$/);
assert.ok(grants[0].toLowerCase().includes(`${userInfo().username.toLowerCase()}:(F)`.toLowerCase()), acl);
} else {
assert.equal(saved.access, 'posix-0600');
assert.equal(statSync(path).mode & 0o777, 0o600);
@@ -109,11 +110,38 @@ test('`call --read-only` refuses mutating tools (what remote hosts execute)', ()
assert.equal(JSON.parse(allowed.stdout).verdict, 'FAIL');
});
test('Windows hosts-file restriction fails closed when icacls fails', () => {
test('Windows hosts-file restriction removes explicit grants and handles literal Unicode paths', { skip: process.platform !== 'win32' }, async () => {
await withHosts(async (path) => {
// Wildcards and PowerShell expressions are legal filename characters. A
// literal path on stdin must neither expand nor execute any of this text.
const literal = join(dirname(path), "hosts-é [x] $(throw 'path-executed').json");
saveHost({ name: 'pi', ssh: 'ruv@pi.local' }, literal);
const grant = spawnSync('icacls', [literal, '/grant', '*S-1-1-0:R', '*S-1-5-18:F'], { encoding: 'utf8' });
assert.equal(grant.status, 0, grant.stderr || grant.stdout);
const before = spawnSync('icacls', [literal], { encoding: 'utf8' });
assert.equal(before.status, 0, before.stderr);
assert.ok(before.stdout.split('\n').filter(line => /:\(/.test(line)).length >= 3, before.stdout);
assert.equal(restrictToOwner(literal), 'windows-acl-owner-only');
const after = spawnSync('icacls', [literal], { encoding: 'utf8' });
assert.equal(after.status, 0, after.stderr);
const grants = after.stdout.split('\n').map(line => line.replace(literal, '').trim()).filter(line => /:\(/.test(line));
assert.equal(grants.length, 1, after.stdout);
assert.ok(grants[0].toLowerCase().includes(`${userInfo().username}:(F)`.toLowerCase()), after.stdout);
assert.equal(JSON.parse(readFileSync(literal, 'utf8')).hosts[0].name, 'pi');
});
});
test('Windows hosts-file restriction fails closed and keeps input out of PowerShell source', () => {
const calls = [];
const ok = restrictToOwner('C:/x/hosts.json', { platform: 'win32', user: () => 'alice', exec: (cmd, args) => { calls.push([cmd, ...args]); return { status: 0 }; } });
const path = "C:/x/hosts $(throw 'injected').json";
const ok = restrictToOwner(path, { platform: 'win32', exec: (cmd, args, options) => { calls.push({ cmd, args, options }); return { status: 0 }; } });
assert.equal(ok, 'windows-acl-owner-only');
assert.deepEqual(calls[0], ['icacls', 'C:/x/hosts.json', '/inheritance:r', '/grant:r', 'alice:F']);
assert.throws(() => restrictToOwner('C:/x/hosts.json', { platform: 'win32', user: () => 'alice', exec: () => ({ status: 5 }) }), /not made private/);
assert.equal(calls[0].cmd, 'powershell.exe');
assert.deepEqual(calls[0].args.slice(0, 4), ['-NoLogo', '-NoProfile', '-NonInteractive', '-Command']);
assert.ok(!calls[0].args.at(-1).includes(path));
assert.deepEqual(JSON.parse(calls[0].options.input), { path });
for (const result of [{ status: 5 }, { status: null, error: { code: 'ENOENT' } }, { status: null, error: { code: 'ETIMEDOUT' } }]) {
assert.throws(() => restrictToOwner(path, { platform: 'win32', exec: () => result }), /not made private/);
}
assert.equal(restrictToOwner('/tmp/h.json', { platform: 'linux', exec: () => { throw new Error('must not run'); } }), 'posix-0600');
});