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
5 changes: 5 additions & 0 deletions .changeset/fix-allowed-paths-schema.md
Original file line number Diff line number Diff line change
@@ -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.
172 changes: 170 additions & 2 deletions src/commands/settings-command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import type {
ResolvedConfig,
} from "../config";
import { configLoader } from "../config";
import { normalizeAllowedPaths } from "../utils/migration";

type FeatureKey = keyof ResolvedConfig["features"];

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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",
),
},
],
Expand Down
12 changes: 9 additions & 3 deletions src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -204,6 +207,11 @@ const migrations: Migration<GuardrailsConfig>[] = [
shouldRun: (config) => needsEnvFilesToPoliciesMigration(config),
run: (config) => migrateEnvFilesToPolicies(config),
},
{
name: "normalize-allowed-paths",
shouldRun: (config) => needsAllowedPathsMigration(config),
run: (config) => migrateAllowedPaths(config),
},
];

const DEFAULT_CONFIG: ResolvedConfig = {
Expand Down Expand Up @@ -374,9 +382,7 @@ export const configLoader = new ConfigLoader<GuardrailsConfig, ResolvedConfig>(
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];

Expand Down
5 changes: 2 additions & 3 deletions src/hooks/path-access.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -246,9 +247,7 @@ async function persistGrant(
unknown
>;
const pa = (raw.pathAccess ?? {}) as Record<string, unknown>;
const existing = Array.isArray(pa.allowedPaths)
? (pa.allowedPaths as string[])
: [];
const existing = normalizeAllowedPaths(pa.allowedPaths);

if (existing.includes(storagePath)) return;

Expand Down
58 changes: 58 additions & 0 deletions src/utils/migration.test.ts
Original file line number Diff line number Diff line change
@@ -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/",
]);
});
});
45 changes: 45 additions & 0 deletions src/utils/migration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, unknown>;
const pathAccess = raw.pathAccess as Record<string, unknown> | 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<string>();
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<string, unknown>).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<string, unknown>;
const pathAccess = migrated.pathAccess as Record<string, unknown> | 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.
*/
Expand Down