diff --git a/.changeset/fix-allowed-paths-schema.md b/.changeset/fix-allowed-paths-schema.md new file mode 100644 index 0000000..2d63e34 --- /dev/null +++ b/.changeset/fix-allowed-paths-schema.md @@ -0,0 +1,5 @@ +--- +"@aliou/pi-guardrails": patch +--- + +Fix path access allowed paths settings to read and write string arrays, and migrate configs written with pattern objects. diff --git a/src/commands/settings-command.ts b/src/commands/settings-command.ts index f3afd95..a8696e0 100644 --- a/src/commands/settings-command.ts +++ b/src/commands/settings-command.ts @@ -27,6 +27,7 @@ import type { ResolvedConfig, } from "../config"; import { configLoader } from "../config"; +import { normalizeAllowedPaths } from "../utils/migration"; type FeatureKey = keyof ResolvedConfig["features"]; @@ -691,6 +692,157 @@ class AddRuleSubmenu implements Component { } } +class PathListEditor implements Component { + private readonly input = new Input(); + private items: string[]; + private selectedIndex = 0; + private mode: "list" | "add" | "edit" = "list"; + private editIndex = -1; + + constructor( + private readonly options: { + label: string; + items: string[]; + theme: SettingsListTheme; + onSave: (items: string[]) => void; + onDone: () => void; + maxVisible?: number; + }, + ) { + this.items = [...options.items]; + this.input.onSubmit = () => this.submit(); + this.input.onEscape = () => this.cancel(); + } + + invalidate() {} + + render(width: number): string[] { + const lines = [ + this.options.theme.label(` ${this.options.label}`, true), + "", + ]; + if (this.mode === "add" || this.mode === "edit") { + lines.push( + this.options.theme.hint( + this.mode === "edit" ? " Edit path:" : " New path:", + ), + "", + ...this.input.render(Math.max(1, width - 4)).map((line) => ` ${line}`), + "", + this.options.theme.hint(" Enter: save · Esc: cancel"), + ); + return lines; + } + + if (this.items.length === 0) { + lines.push(this.options.theme.hint(" (empty)")); + } else { + const maxVisible = this.options.maxVisible ?? 10; + const startIndex = Math.max( + 0, + Math.min( + this.selectedIndex - Math.floor(maxVisible / 2), + this.items.length - maxVisible, + ), + ); + const endIndex = Math.min(startIndex + maxVisible, this.items.length); + for (let i = startIndex; i < endIndex; i++) { + const item = this.items[i]; + if (!item) continue; + const isSelected = i === this.selectedIndex; + const prefix = isSelected ? this.options.theme.cursor : " "; + lines.push(prefix + this.options.theme.value(item, isSelected)); + } + if (startIndex > 0 || endIndex < this.items.length) { + lines.push( + this.options.theme.hint( + ` (${this.selectedIndex + 1}/${this.items.length})`, + ), + ); + } + } + + lines.push(""); + lines.push( + this.options.theme.hint( + " a: add · e/Enter: edit · d: delete · Esc: back", + ), + ); + return lines; + } + + handleInput(data: string): void { + if (this.mode === "add" || this.mode === "edit") { + this.input.handleInput(data); + return; + } + + if (matchesKey(data, Key.up) || data === "k") { + if (this.items.length === 0) return; + this.selectedIndex = + this.selectedIndex === 0 + ? this.items.length - 1 + : this.selectedIndex - 1; + } else if (matchesKey(data, Key.down) || data === "j") { + if (this.items.length === 0) return; + this.selectedIndex = + this.selectedIndex === this.items.length - 1 + ? 0 + : this.selectedIndex + 1; + } else if (data === "a" || data === "A") { + this.mode = "add"; + this.input.setValue(""); + } else if (data === "e" || data === "E" || matchesKey(data, Key.enter)) { + this.startEdit(); + } else if (data === "d" || data === "D") { + this.deleteSelected(); + } else if (matchesKey(data, Key.escape)) { + this.options.onDone(); + } + } + + private startEdit(): void { + const item = this.items[this.selectedIndex]; + if (!item) return; + this.mode = "edit"; + this.editIndex = this.selectedIndex; + this.input.setValue(item); + } + + private submit(): void { + const path = this.input.getValue().trim(); + if (!path) { + this.cancel(); + return; + } + + if (this.mode === "edit") { + this.items[this.editIndex] = path; + } else { + this.items.push(path); + this.selectedIndex = this.items.length - 1; + } + this.items = [...new Set(this.items)]; + this.options.onSave([...this.items]); + this.cancel(); + } + + private deleteSelected(): void { + if (this.items.length === 0) return; + this.items.splice(this.selectedIndex, 1); + if (this.selectedIndex >= this.items.length) { + this.selectedIndex = Math.max(0, this.items.length - 1); + } + this.options.onSave([...this.items]); + } + + private cancel(): void { + this.mode = "list"; + this.editIndex = -1; + this.input.setValue(""); + } +} + class ScopePickerSubmenu implements Component { private selectedIndex = 0; @@ -1067,6 +1219,23 @@ export function registerGuardrailsSettings(pi: ExtensionAPI): void { }; } + function pathListSubmenu(id: string, label: string) { + return (_val: string, submenuDone: (v?: string) => void) => { + const items = normalizeAllowedPaths(getNestedValue(scopedConfig, id)); + let latestCount = items.length; + return new PathListEditor({ + label, + items, + theme: settingsTheme, + onSave: (newItems) => { + latestCount = newItems.length; + applyDraft(id, newItems); + }, + onDone: () => submenuDone(`${latestCount} items`), + }); + }; + } + function patternConfigSubmenu( id: string, label: string, @@ -1231,10 +1400,9 @@ export function registerGuardrailsSettings(pi: ExtensionAPI): void { description: "Paths always allowed (trailing / for directories). Supports ~/", currentValue: count("pathAccess.allowedPaths"), - submenu: patternConfigSubmenu( + submenu: pathListSubmenu( "pathAccess.allowedPaths", "Allowed Paths", - "file", ), }, ], diff --git a/src/config.ts b/src/config.ts index 2b604d5..e15ae45 100644 --- a/src/config.ts +++ b/src/config.ts @@ -137,10 +137,13 @@ import { ConfigLoader, type Migration } from "@aliou/pi-utils-settings"; import { backupConfig, CURRENT_VERSION, + migrateAllowedPaths, migrateEnvFilesToPolicies, migrateV0, + needsAllowedPathsMigration, needsEnvFilesToPoliciesMigration, needsMigration, + normalizeAllowedPaths, } from "./utils/migration"; import { pendingWarnings } from "./utils/warnings"; @@ -204,6 +207,11 @@ const migrations: Migration[] = [ shouldRun: (config) => needsEnvFilesToPoliciesMigration(config), run: (config) => migrateEnvFilesToPolicies(config), }, + { + name: "normalize-allowed-paths", + shouldRun: (config) => needsAllowedPathsMigration(config), + run: (config) => migrateAllowedPaths(config), + }, ]; const DEFAULT_CONFIG: ResolvedConfig = { @@ -374,9 +382,7 @@ export const configLoader = new ConfigLoader( local?.pathAccess?.allowedPaths, memory?.pathAccess?.allowedPaths, ]) { - if (paths) { - for (const p of paths) mergedPaths.add(p); - } + for (const p of normalizeAllowedPaths(paths)) mergedPaths.add(p); } resolved.pathAccess.allowedPaths = [...mergedPaths]; diff --git a/src/hooks/path-access.ts b/src/hooks/path-access.ts index 17e1000..5e28352 100644 --- a/src/hooks/path-access.ts +++ b/src/hooks/path-access.ts @@ -12,6 +12,7 @@ import { import { configLoader } from "../config"; import { extractBashPathCandidates } from "../utils/bash-paths"; import { emitBlocked } from "../utils/events"; +import { normalizeAllowedPaths } from "../utils/migration"; import { normalizeForDisplay, resolveFromCwd, @@ -246,9 +247,7 @@ async function persistGrant( unknown >; const pa = (raw.pathAccess ?? {}) as Record; - const existing = Array.isArray(pa.allowedPaths) - ? (pa.allowedPaths as string[]) - : []; + const existing = normalizeAllowedPaths(pa.allowedPaths); if (existing.includes(storagePath)) return; diff --git a/src/utils/migration.test.ts b/src/utils/migration.test.ts new file mode 100644 index 0000000..ad5eccc --- /dev/null +++ b/src/utils/migration.test.ts @@ -0,0 +1,58 @@ +import { describe, expect, it } from "vitest"; +import type { GuardrailsConfig } from "../config"; +import { + migrateAllowedPaths, + needsAllowedPathsMigration, + normalizeAllowedPaths, +} from "./migration"; + +describe("allowedPaths migration", () => { + it("normalizes strings and legacy pattern objects", () => { + expect( + normalizeAllowedPaths([ + "/dev/null", + { pattern: "~/Downloads/" }, + { pattern: " /tmp/file " }, + { pattern: "" }, + { regex: true }, + 42, + null, + "/dev/null", + ]), + ).toEqual(["/dev/null", "~/Downloads/", "/tmp/file"]); + }); + + it("detects legacy object-shaped allowed paths", () => { + const config = { + pathAccess: { + allowedPaths: [{ pattern: "/dev/null" }], + }, + } as unknown as GuardrailsConfig; + + expect(needsAllowedPathsMigration(config)).toBe(true); + }); + + it("does not migrate valid string allowed paths", () => { + const config: GuardrailsConfig = { + pathAccess: { + allowedPaths: ["/dev/null"], + }, + }; + + expect(needsAllowedPathsMigration(config)).toBe(false); + }); + + it("converts legacy object-shaped allowed paths to strings", () => { + const config = { + pathAccess: { + mode: "block", + allowedPaths: [{ pattern: "/dev/null" }, "~/Downloads/"], + }, + } as unknown as GuardrailsConfig; + + expect(migrateAllowedPaths(config).pathAccess?.allowedPaths).toEqual([ + "/dev/null", + "~/Downloads/", + ]); + }); +}); diff --git a/src/utils/migration.ts b/src/utils/migration.ts index 87ecfc6..088bd14 100644 --- a/src/utils/migration.ts +++ b/src/utils/migration.ts @@ -151,6 +151,51 @@ export function migrateMarkOnboardingDone( return migrated; } +/** + * Migrate allowedPaths entries accidentally written as PatternConfig objects. + */ +export function needsAllowedPathsMigration(config: GuardrailsConfig): boolean { + const raw = config as Record; + const pathAccess = raw.pathAccess as Record | undefined; + if (!Array.isArray(pathAccess?.allowedPaths)) return false; + return pathAccess.allowedPaths.some((item) => typeof item !== "string"); +} + +export function normalizeAllowedPaths(items: unknown): string[] { + if (!Array.isArray(items)) return []; + + const paths = new Set(); + for (const item of items) { + let path: string | null = null; + if (typeof item === "string") { + path = item; + } else if (typeof item === "object" && item !== null) { + const pattern = (item as Record).pattern; + if (typeof pattern === "string") path = pattern; + } + + const normalized = path?.trim(); + if (normalized) paths.add(normalized); + } + + return [...paths]; +} + +export function migrateAllowedPaths( + config: GuardrailsConfig, +): GuardrailsConfig { + const migrated = structuredClone(config) as Record; + const pathAccess = migrated.pathAccess as Record | undefined; + if (!pathAccess) return migrated as GuardrailsConfig; + + pathAccess.allowedPaths = normalizeAllowedPaths(pathAccess.allowedPaths); + migrated.version = CURRENT_VERSION; + pendingWarnings.push( + "[guardrails] pathAccess.allowedPaths was migrated from pattern objects to path strings.", + ); + return migrated as GuardrailsConfig; +} + /** * Migrate deprecated envFiles/protectEnvFiles fields to policies. */