mirror of
https://github.com/ruvnet/ruflo.git
synced 2026-11-02 18:17:51 +09:00
* fix(console): accept Windows paths in the autopilot envelope #4434 added Windows path parity across the console, but the autopilot envelope's own path validator still rejected any `\`, and the downstream `isProtectedPath` and `pathAllowed` guards compared with POSIX-only string ops. The code is unreachable today because autopilot start is still gated off on Windows (ap-windows-gate.spec.ts), but it is the next thing a Windows user will trip on when that gate is lifted — the natural `C:\work\app` form, and every UNC path, is refused with the opaque "is not an absolute folder below the root with no .." error. Four narrow fixes in `hooks/data/ap-envelope.ts`, driven by the separator-aware helpers already in `hooks/data/paths.ts`: - `isProtectedPath`: fold `\` to `/` on a drive-lettered or UNC path before the protected-dir compare. - `validateEnvelope` path check: drop the unconditional `path.includes('\')` reject (it contradicted `isAbsolutePath` which already accepts `C:\x` and `\srv\share`), and tighten the drive-root regex `/^[A-Za-z]:\/?$/` → `/^[A-Za-z]:[\/]?$/` so `C:\` is still refused explicitly (was previously caught by the backslash reject). - `validateEnvelope` output canonicalisation: use `trimTrailing` instead of `/\/+$/` replace, so a trailing `\` on a Windows path is stripped the way `/` already is on POSIX. - `pathAllowed`: use `hasParentSegment`, `samePath` and `below` from paths.ts. A file at `C:\work\app\file.ts` is now inside envelope path `C:\work\app` (and `c:\WORK\app\file.ts` is too), while `C:\work\application\file.ts` still is not. Semantics on a POSIX envelope are unchanged — the new helpers degrade exactly to their old string ops when neither side is a Windows path. New `tests/ap-envelope-windows-paths.spec.ts` pins what should and should not be accepted (13 cases: backslash drive, forward-slash drive, UNC, trailing-separator strip, `..`, bare-drive-root, protected-folder, pathAllowed under case + separator variation, prefix-match rejection). Verified on Windows 11: new spec 13/13, ap-core.spec.ts 31/31 (no POSIX regression), 5 envelope-adjacent specs 64/64 combined, full console suite 3444 total · 3399 pass · 13 fail — same pre-existing baseline as #4434's own run, zero regressions. Follow-up to #4434 R1 (https://github.com/ruvnet/ruflo/pull/4434#issuecomment-6371423874). Co-Authored-By: RuFlo <ruv@ruv.net> * fix: keep the backslash reject for POSIX-rooted envelope paths #4436 review (@ruvnet): dropping `path.includes('\')` outright also dropped the refusal for a POSIX-rooted path with backslashes smuggled in. `/work/app\..\..\other` passes `isAbsolutePath` (POSIX form), and `hasParentSegment` splits POSIX-only when `isWindowsPath` says the path is not Windows, so the `..` segments are invisible to it. On a Windows host, that leading-`/` path then resolves to `\other`, outside the folder the user named. Unreachable today because autopilot is still gated off on Windows, but it is the guard that gate's removal depends on. Keep the backslash reject, but only for non-Windows paths (POSIX should never have `\` in its path). The native Windows forms `C:\work\app` and `\srv\share\app` continue to pass; the mixed `/work/app\..\..\other` form is refused as before. Added case to tests/ap-envelope-windows-paths.spec.ts. Co-Authored-By: RuFlo <ruv@ruv.net> --------- Co-authored-by: RuFlo <ruv@ruv.net>
This commit is contained in:
1 parent
1d6930e583
commit
43a3954350
2 files changed
+157
-7
No files matched your search
@@ -4,7 +4,7 @@
|
||||
* hash does not match its body is treated as tampered and the loop refuses to run on it. Hard denies are not part of the envelope: they
|
||||
* are a constant of this file, and an envelope that names one as allowed is rejected, never trimmed.
|
||||
*/
|
||||
import { hasParentSegment, isAbsolutePath, isWindowsPath } from './paths'
|
||||
import { below, hasParentSegment, isAbsolutePath, isWindowsPath, samePath, trimTrailing } from './paths'
|
||||
|
||||
/** Classes of action an envelope can allow. Each task is classified (data/ap-guard.ts) into exactly one, or is parked. */
|
||||
export const TOOL_CLASSES = ['read', 'edit', 'test', 'git-local', 'git-branch', 'network', 'spawn', 'mcp'] as const
|
||||
@@ -28,8 +28,10 @@ export const DENY_PATTERNS: readonly [HardDeny, RegExp][] = [
|
||||
/** Folders the autopilot's own steps may never be pointed at: its envelope, journal and kill flag, and Project Anatole's status. A task or envelope that names one is refused, not trimmed. */
|
||||
export const PROTECTED_DIRS = ['.claude-flow/console', '.claude-flow/protector-mod'] as const
|
||||
export const isProtectedPath = (path: string): boolean => {
|
||||
// Windows file systems ignore case, so a drive-lettered path is compared lower-cased; a POSIX path stays exact.
|
||||
const flat = (isWindowsPath(path) ? path.toLowerCase() : path).replace(/\/+/g, '/').replace(/\/\.\//g, '/')
|
||||
// Windows file systems ignore case and either separator, so a drive-lettered or UNC path has its `\` folded to `/`
|
||||
// and is lower-cased before compare; a POSIX path stays exact. Doubled separators and `/./` segments are flattened.
|
||||
const windows = isWindowsPath(path)
|
||||
const flat = (windows ? path.replace(/\\/g, '/').toLowerCase() : path).replace(/\/+/g, '/').replace(/\/\.\//g, '/')
|
||||
|
||||
return PROTECTED_DIRS.some(dir => flat.includes(dir)) || flat.endsWith('/.claude-flow') || flat.endsWith('/.claude-flow/')
|
||||
}
|
||||
@@ -120,7 +122,7 @@ export function validateEnvelope(raw: unknown): Checked {
|
||||
if (paths === null || paths.length === 0) errors.push('paths: at least one absolute folder')
|
||||
|
||||
for (const path of paths ?? []) {
|
||||
if (!isAbsolutePath(path) || path === '/' || /^[A-Za-z]:\/?$/.test(path) || path.length > 300 || BAD_CHARS.test(path) || hasParentSegment(path) || path.includes('\\')) errors.push(`paths: "${path.slice(0, 40)}" is not an absolute folder below the root with no ..`)
|
||||
if (!isAbsolutePath(path) || path === '/' || /^[A-Za-z]:[\\/]?$/.test(path) || path.length > 300 || BAD_CHARS.test(path) || hasParentSegment(path) || (path.includes('\\') && !isWindowsPath(path))) errors.push(`paths: "${path.slice(0, 40)}" is not an absolute folder below the root with no ..`)
|
||||
else if (isProtectedPath(path)) errors.push(`paths: "${path.slice(0, 40)}" is the autopilot's or Project Anatole's own folder: steps are never pointed at it`)
|
||||
}
|
||||
|
||||
@@ -175,7 +177,7 @@ export function validateEnvelope(raw: unknown): Checked {
|
||||
envelope: {
|
||||
name: name as string,
|
||||
toolClasses: [...new Set(classes as ToolClass[])].sort(),
|
||||
paths: [...new Set(paths as string[])].map(path => path.replace(/\/+$/, '')).sort(),
|
||||
paths: [...new Set(paths as string[])].map(trimTrailing).sort(),
|
||||
repos: [...new Set(repos as string[])].sort(),
|
||||
network: [...new Set(network as string[])].sort(),
|
||||
secretEnv: [...new Set(secretEnv as string[])].sort(),
|
||||
@@ -335,8 +337,8 @@ export function narrowed(prev: Envelope, next: Envelope): string[] {
|
||||
return out
|
||||
}
|
||||
|
||||
/** A path is inside the envelope when it is one of its folders or below one (compared at a folder boundary, so /a/bc is not inside /a/b). */
|
||||
export const pathAllowed = (envelope: Envelope, path: string): boolean => !path.split('/').includes('..') && !isProtectedPath(path) && envelope.paths.some(root => path === root || path.startsWith(`${root}/`))
|
||||
/** A path is inside the envelope when it is one of its folders or below one (compared at a folder boundary, so /a/bc is not inside /a/b). Windows paths compare ignoring case and separator style, so `C:\work\app\file.ts` is inside envelope path `C:\work\app`. */
|
||||
export const pathAllowed = (envelope: Envelope, path: string): boolean => !hasParentSegment(path) && !isProtectedPath(path) && envelope.paths.some(root => samePath(root, path) || below(root, path) !== null)
|
||||
|
||||
export const hostAllowed = (envelope: Envelope, host: string): boolean => envelope.network.includes(host.toLowerCase())
|
||||
|
||||
|
||||
@@ -0,0 +1,148 @@
|
||||
/**
|
||||
* The autopilot envelope's path checks on Windows. #4434 added Windows path parity across the console but the envelope's own path
|
||||
* validator still refused any `\`, and `pathAllowed` compared with POSIX-only string ops; so `C:\work\app` and `\\srv\share\app` were
|
||||
* unusable and a file at `C:\work\app\file.ts` was NOT inside envelope path `C:\work\app`. This spec pins down:
|
||||
* - validateEnvelope accepts both `C:\...` and `C:/...` forms, and UNC paths
|
||||
* - canonical output strips a trailing `\` on a Windows path (same semantics as it strips `/` on a POSIX one)
|
||||
* - isProtectedPath catches the two protected directories whichever separator the path uses
|
||||
* - pathAllowed matches a file inside a Windows envelope path ignoring case and separator style
|
||||
* - the drive-letter root check still refuses `C:\` and `C:/` and `C:`
|
||||
* Run with
|
||||
* npx vitest run plugins/ruflo-console/tests/ap-envelope-windows-paths.spec.ts
|
||||
*/
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import { isProtectedPath, pathAllowed, validateEnvelope, type Envelope } from '../hooks/data/ap-envelope'
|
||||
|
||||
const H = 3_600_000
|
||||
|
||||
const envWith = (paths: string[]): unknown => ({
|
||||
name: 'win shift',
|
||||
toolClasses: ['edit', 'read', 'test'],
|
||||
paths,
|
||||
repos: [],
|
||||
network: [],
|
||||
secretEnv: [],
|
||||
spend: { hourUsd: 2, dayUsd: 10, totalUsd: 40 },
|
||||
concurrency: 2,
|
||||
maxDurationMs: 7 * 24 * H,
|
||||
verify: [['true']],
|
||||
acceptWithoutAnatole: false,
|
||||
})
|
||||
|
||||
describe('validateEnvelope on Windows paths', () => {
|
||||
it('accepts a native backslash drive path', () => {
|
||||
const result = validateEnvelope(envWith(['C:\\work\\app']))
|
||||
|
||||
expect(result.ok).toBe(true)
|
||||
if (result.ok) expect(result.envelope.paths).toEqual(['C:\\work\\app'])
|
||||
})
|
||||
|
||||
it('accepts a forward-slash drive path', () => {
|
||||
const result = validateEnvelope(envWith(['C:/work/app']))
|
||||
|
||||
expect(result.ok).toBe(true)
|
||||
if (result.ok) expect(result.envelope.paths).toEqual(['C:/work/app'])
|
||||
})
|
||||
|
||||
it('accepts a UNC share path', () => {
|
||||
const result = validateEnvelope(envWith(['\\\\server\\share\\app']))
|
||||
|
||||
expect(result.ok).toBe(true)
|
||||
if (result.ok) expect(result.envelope.paths).toEqual(['\\\\server\\share\\app'])
|
||||
})
|
||||
|
||||
it('still refuses the bare drive root in either separator', () => {
|
||||
for (const bad of ['C:', 'C:\\', 'C:/']) {
|
||||
const result = validateEnvelope(envWith([bad]))
|
||||
|
||||
expect(result.ok).toBe(false)
|
||||
if (!result.ok) expect(result.errors.some(e => e.startsWith('paths'))).toBe(true)
|
||||
}
|
||||
})
|
||||
|
||||
it('still refuses .. in a backslash path', () => {
|
||||
const result = validateEnvelope(envWith(['C:\\work\\..\\etc']))
|
||||
|
||||
expect(result.ok).toBe(false)
|
||||
if (!result.ok) expect(result.errors.some(e => e.startsWith('paths'))).toBe(true)
|
||||
})
|
||||
|
||||
it('still refuses a POSIX-rooted path that smuggles backslashes (hasParentSegment would miss them)', () => {
|
||||
// #4436 review: `/work/app\..\..\other` passes isAbsolutePath (POSIX) and hasParentSegment splits POSIX-only,
|
||||
// so without a `\` reject for non-Windows paths the `..` segments are invisible to the validator.
|
||||
for (const bad of ['/work/app\\..\\..\\other', '/work/app\\sub', '/work/app\\..']) {
|
||||
const result = validateEnvelope(envWith([bad]))
|
||||
|
||||
expect(result.ok).toBe(false)
|
||||
if (!result.ok) expect(result.errors.some(e => e.startsWith('paths'))).toBe(true)
|
||||
}
|
||||
})
|
||||
|
||||
it('strips a trailing backslash from a Windows path, same as it strips a trailing slash on POSIX', () => {
|
||||
const result = validateEnvelope(envWith(['C:\\work\\app\\']))
|
||||
|
||||
expect(result.ok).toBe(true)
|
||||
if (result.ok) expect(result.envelope.paths).toEqual(['C:\\work\\app'])
|
||||
})
|
||||
|
||||
it('refuses a Windows path that names the autopilot folder, whichever separator', () => {
|
||||
for (const bad of ['C:\\work\\app\\.claude-flow\\console', 'C:/work/app/.claude-flow/console']) {
|
||||
const result = validateEnvelope(envWith([bad]))
|
||||
|
||||
expect(result.ok).toBe(false)
|
||||
if (!result.ok) expect(result.errors.some(e => /autopilot's or Project Anatole/.test(e))).toBe(true)
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
describe('isProtectedPath on Windows', () => {
|
||||
it('catches the protected dirs through either separator and either case', () => {
|
||||
expect(isProtectedPath('C:\\Users\\me\\app\\.claude-flow\\console')).toBe(true)
|
||||
expect(isProtectedPath('C:\\Users\\ME\\app\\.CLAUDE-FLOW\\CONSOLE')).toBe(true)
|
||||
expect(isProtectedPath('C:/Users/me/app/.claude-flow/protector-mod')).toBe(true)
|
||||
expect(isProtectedPath('\\\\srv\\share\\.claude-flow\\console')).toBe(true)
|
||||
})
|
||||
|
||||
it('does not catch an unrelated Windows path', () => {
|
||||
expect(isProtectedPath('C:\\work\\app\\src\\index.ts')).toBe(false)
|
||||
expect(isProtectedPath('\\\\srv\\share\\app\\src\\index.ts')).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe('pathAllowed on Windows', () => {
|
||||
const ENV: Envelope = {
|
||||
name: 'win',
|
||||
toolClasses: ['edit', 'read', 'test'],
|
||||
paths: ['C:\\work\\app'],
|
||||
repos: [],
|
||||
network: [],
|
||||
secretEnv: [],
|
||||
spend: { hourUsd: 2, dayUsd: 10, totalUsd: 40 },
|
||||
concurrency: 2,
|
||||
maxDurationMs: 7 * 24 * H,
|
||||
verify: [['true']],
|
||||
acceptWithoutAnatole: false,
|
||||
}
|
||||
|
||||
it('matches a file inside the envelope folder ignoring case and separator', () => {
|
||||
expect(pathAllowed(ENV, 'C:\\work\\app\\file.ts')).toBe(true)
|
||||
expect(pathAllowed(ENV, 'C:/work/app/src/index.ts')).toBe(true)
|
||||
expect(pathAllowed(ENV, 'c:\\WORK\\app\\file.ts')).toBe(true)
|
||||
expect(pathAllowed(ENV, 'C:\\work\\app')).toBe(true)
|
||||
})
|
||||
|
||||
it('refuses a file whose folder is only a prefix of the envelope folder', () => {
|
||||
expect(pathAllowed(ENV, 'C:\\work\\application\\file.ts')).toBe(false)
|
||||
expect(pathAllowed(ENV, 'C:\\work\\appother\\file.ts')).toBe(false)
|
||||
})
|
||||
|
||||
it('refuses a path with .. in either separator', () => {
|
||||
expect(pathAllowed(ENV, 'C:\\work\\app\\..\\etc')).toBe(false)
|
||||
expect(pathAllowed(ENV, 'C:/work/app/../etc')).toBe(false)
|
||||
})
|
||||
|
||||
it('refuses a protected folder below the envelope path', () => {
|
||||
expect(pathAllowed(ENV, 'C:\\work\\app\\.claude-flow\\console')).toBe(false)
|
||||
})
|
||||
})
|
||||
Reference in new issue
Block a user