Skip to content

feat: check redirect targets against edit rules and external_directory #14

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
preved911 merged 1 commit into from
Jul 29, 2026
Merged

feat: check redirect targets against edit rules and external_directory #14

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
48 changes: 48 additions & 0 deletions src/__tests__/chain.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,4 +97,52 @@ describe("parseChain", () => {
expect(result.segments).toHaveLength(0);
expect(result.parseError).toBe(false);
});

it("captures fd redirect as well-known", () => {
const result = parseChain("ls -la 2>&1");
expect(result.segments).toHaveLength(1);
expect(result.segments[0].redirects).toHaveLength(1);
expect(result.segments[0].redirects[0].target).toBe("1");
expect(result.segments[0].redirects[0].wellKnown).toBe(true);
expect(result.segments[0].command).toContain("2>&1");
});

it("captures /dev/null redirect as well-known", () => {
const result = parseChain("ls -la > /dev/null");
expect(result.segments).toHaveLength(1);
expect(result.segments[0].redirects).toHaveLength(1);
expect(result.segments[0].redirects[0].target).toBe("/dev/null");
expect(result.segments[0].redirects[0].wellKnown).toBe(true);
expect(result.segments[0].command).toContain(">/dev/null");
});

it("captures file redirect as not well-known", () => {
const result = parseChain("ls -la > /tmp/out.txt");
expect(result.segments).toHaveLength(1);
expect(result.segments[0].redirects).toHaveLength(1);
expect(result.segments[0].redirects[0].target).toBe("/tmp/out.txt");
expect(result.segments[0].redirects[0].wellKnown).toBe(false);
expect(result.segments[0].command).toContain(">/tmp/out.txt");
});

it("captures heredoc as well-known", () => {
const result = parseChain("cat << EOF");
expect(result.segments).toHaveLength(1);
expect(result.segments[0].redirects).toHaveLength(1);
expect(result.segments[0].redirects[0].wellKnown).toBe(true);
});

it("captures redirect in chain", () => {
const result = parseChain("echo hello > file.txt && cat file.txt");
expect(result.segments).toHaveLength(2);
expect(result.segments[0].redirects).toHaveLength(1);
expect(result.segments[0].redirects[0].target).toBe("file.txt");
expect(result.segments[0].redirects[0].wellKnown).toBe(false);
expect(result.segments[1].redirects).toHaveLength(0);
});

it("includes redirect in command text", () => {
const result = parseChain("echo test 2>/dev/null");
expect(result.segments[0].command).toBe("echo test 2>/dev/null");
});
});
126 changes: 119 additions & 7 deletions src/__tests__/enforce.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
import { describe, it, expect, beforeEach } from "vitest";
import { resolveSegment, resolveChain, beforeExecute, handlePermissionAsk, clearStoredDecision, isComplexChain, buildReadabilityMessage } from "../enforce.js";
import type { PluginConfig } from "../config.js";
import type { ChainSegment } from "../chain.js";

const defaultConfig: PluginConfig = {
bashRules: [
{ pattern: "*", action: "ask" },
{ pattern: "git *", action: "allow" },
{ pattern: "sudo *", action: "deny" },
],
editRules: [],
externalDirectoryRules: [
{ pattern: "./**", action: "allow" },
],
Expand All @@ -26,6 +28,7 @@ describe("resolveSegment", () => {
const configNoMatch: PluginConfig = {
...defaultConfig,
bashRules: [{ pattern: "*", action: "ask" }],
editRules: [],
};
const act = resolveSegment("cat /etc/passwd", "cat", "/project", configNoMatch);
expect(act).not.toBeNull();
Expand All @@ -34,6 +37,7 @@ describe("resolveSegment", () => {
it("most restrictive wins across checks", () => {
const config: PluginConfig = {
bashRules: [{ pattern: "*", action: "ask" }],
editRules: [],
externalDirectoryRules: [{ pattern: "*", action: "deny" }],
externalDirectoryDefault: null,
enabled: true,
Expand All @@ -45,6 +49,7 @@ describe("resolveSegment", () => {
it("no check triggers returns null", () => {
const config: PluginConfig = {
bashRules: [],
editRules: [],
externalDirectoryRules: [],
externalDirectoryDefault: null,
enabled: true,
Expand All @@ -58,8 +63,8 @@ describe("resolveChain", () => {
it("all segments allowed — chain let through", () => {
const chain = resolveChain(
[
{ command: "git status", commandName: "git" },
{ command: "git log", commandName: "git" },
{ command: "git status", commandName: "git", redirects: [] },
{ command: "git log", commandName: "git", redirects: [] },
],
"/project",
defaultConfig,
Expand All @@ -70,14 +75,15 @@ describe("resolveChain", () => {
it("any segment not allowed — chain takes its action", () => {
const config: PluginConfig = {
bashRules: [{ pattern: "git *", action: "allow" }],
editRules: [],
externalDirectoryRules: [],
externalDirectoryDefault: null,
enabled: true,
};
const chain = resolveChain(
[
{ command: "git status", commandName: "git" },
{ command: "rm -rf /", commandName: "rm" },
{ command: "git status", commandName: "git", redirects: [] },
{ command: "rm -rf /", commandName: "rm", redirects: [] },
],
"/project",
config,
Expand All @@ -88,8 +94,8 @@ describe("resolveChain", () => {
it("deny in any segment denies whole chain", () => {
const chain = resolveChain(
[
{ command: "git status", commandName: "git" },
{ command: "sudo rm -rf /", commandName: "sudo" },
{ command: "git status", commandName: "git", redirects: [] },
{ command: "sudo rm -rf /", commandName: "sudo", redirects: [] },
],
"/project",
defaultConfig,
Expand All @@ -99,7 +105,7 @@ describe("resolveChain", () => {

it("single segment with no issues", () => {
const chain = resolveChain(
[{ command: "git status", commandName: "git" }],
[{ command: "git status", commandName: "git", redirects: [] }],
"/project",
defaultConfig,
);
Expand Down Expand Up @@ -156,6 +162,7 @@ describe("handlePermissionAsk", () => {
it("does nothing for stored ask decisions", () => {
const config: PluginConfig = {
bashRules: [{ pattern: "*", action: "ask" }],
editRules: [],
externalDirectoryRules: [],
externalDirectoryDefault: null,
enabled: true,
Expand Down Expand Up @@ -208,6 +215,7 @@ describe("readabilityRejection", () => {
bashRules: [
{ pattern: "*", action: "ask" },
],
editRules: [],
externalDirectoryRules: [{ pattern: "./**", action: "allow" }],
externalDirectoryDefault: null,
enabled: true,
Expand All @@ -219,6 +227,7 @@ describe("readabilityRejection", () => {
{ pattern: "git *", action: "allow" },
{ pattern: "npm *", action: "allow" },
],
editRules: [],
externalDirectoryRules: [{ pattern: "./**", action: "allow" }],
externalDirectoryDefault: null,
enabled: true,
Expand Down Expand Up @@ -257,6 +266,7 @@ describe("readabilityRejection", () => {
it("does not reject complex chain when all segments are denied anyway", () => {
const denyConfig: PluginConfig = {
bashRules: [{ pattern: "*", action: "deny" }],
editRules: [],
externalDirectoryRules: [],
externalDirectoryDefault: null,
enabled: true,
Expand Down Expand Up @@ -289,3 +299,105 @@ describe("buildReadabilityMessage", () => {
expect(msg).toContain("exit 1");
});
});

describe("redirect enforcement", () => {
const cwd = "/project";

it("well-known fd redirect does not trigger edit check", () => {
const config: PluginConfig = {
bashRules: [{ pattern: "*", action: "allow" }],
editRules: [{ pattern: "*", action: "deny" }],
externalDirectoryRules: [],
externalDirectoryDefault: null,
enabled: true,
};
const action = resolveSegment("ls", "ls", cwd, config, [
{ operator: ">&", target: "1", fileDescriptor: 2, wellKnown: true },
]);
expect(action).toBe("allow");
});

it("/dev/null redirect does not trigger edit check", () => {
const config: PluginConfig = {
bashRules: [{ pattern: "*", action: "allow" }],
editRules: [{ pattern: "*", action: "deny" }],
externalDirectoryRules: [],
externalDirectoryDefault: null,
enabled: true,
};
const action = resolveSegment("ls", "ls", cwd, config, [
{ operator: ">", target: "/dev/null", fileDescriptor: undefined, wellKnown: true },
]);
expect(action).toBe("allow");
});

it("file redirect inside cwd checks only edit rules", () => {
const config: PluginConfig = {
bashRules: [{ pattern: "*", action: "allow" }],
editRules: [{ pattern: "/project/**", action: "allow" }],
externalDirectoryRules: [{ pattern: "*", action: "deny" }],
externalDirectoryDefault: null,
enabled: true,
};
const action = resolveSegment("ls", "ls", cwd, config, [
{ operator: ">", target: "output.txt", fileDescriptor: undefined, wellKnown: false },
]);
expect(action).toBe("allow");
});

it("file redirect outside cwd checks both edit and external_directory", () => {
const config: PluginConfig = {
bashRules: [{ pattern: "*", action: "allow" }],
editRules: [{ pattern: "/etc/**", action: "deny" }],
externalDirectoryRules: [{ pattern: "./**", action: "allow" }],
externalDirectoryDefault: "ask",
enabled: true,
};
const action = resolveSegment("ls", "ls", cwd, config, [
{ operator: ">", target: "/etc/passwd", fileDescriptor: undefined, wellKnown: false },
]);
expect(action).toBe("deny");
});

it("file redirect outside cwd with denied external_directory", () => {
const config: PluginConfig = {
bashRules: [{ pattern: "*", action: "allow" }],
editRules: [],
externalDirectoryRules: [],
externalDirectoryDefault: "deny",
enabled: true,
};
const action = resolveSegment("ls", "ls", cwd, config, [
{ operator: ">", target: "/tmp/foo", fileDescriptor: undefined, wellKnown: false },
]);
expect(action).toBe("deny");
});

it("redirect with ask edit rule produces ask", () => {
const config: PluginConfig = {
bashRules: [{ pattern: "*", action: "allow" }],
editRules: [{ pattern: "*", action: "ask" }],
externalDirectoryRules: [],
externalDirectoryDefault: null,
enabled: true,
};
const action = resolveSegment("ls", "ls", cwd, config, [
{ operator: ">", target: "out.txt", fileDescriptor: undefined, wellKnown: false },
]);
expect(action).toBe("ask");
});

it("redirect check combined with bash deny still denies", () => {
const config: PluginConfig = {
bashRules: [{ pattern: "*", action: "deny" }],
editRules: [{ pattern: "*", action: "allow" }],
externalDirectoryRules: [],
externalDirectoryDefault: null,
enabled: true,
};
const action = resolveSegment("sudo rm -rf /", "sudo rm -rf /", cwd, config, [
{ operator: ">", target: "out.txt", fileDescriptor: undefined, wellKnown: false },
]);
expect(action).toBe("deny");
});
});
48 changes: 43 additions & 5 deletions src/chain.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,17 @@
import { parse } from "unbash";
import type { Script, Statement, Node, CommandExpansionPart, Command, AndOr, Pipeline } from "unbash";
import type { Script, Statement, Node, CommandExpansionPart, Command, AndOr, Pipeline, Redirect } from "unbash";

export interface RedirectInfo {
operator: string;
target: string;
fileDescriptor: number | undefined;
wellKnown: boolean;
}

export interface ChainSegment {
command: string;
commandName: string;
redirects: RedirectInfo[];
}

export interface ChainResult {
Expand All @@ -13,6 +21,23 @@ export interface ChainResult {
errors: string[];
}

function isWellKnownRedirect(redir: Redirect): boolean {
const target = redir.target?.text ?? redir.content ?? "";
if (target === "/dev/null") return true;
if (/^\d+$/.test(target)) return true;

@preved911 preved911 Jul 29, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is matching for this rule?

@preved911 preved911 Jul 29, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Matches targets that are purely numeric — like the 1 in 2>&1. The idea is that FD redirect targets (e.g. 1, 2) are always single/all digits.

However, you are right to flag this — > 12345 (redirect to a file literally named 12345) would also match. I can tighten the check to only mark it well-known when fileDescriptor is explicitly set AND the target is all digits, which eliminates the ambiguity. Want me to make that change?

@preved911 preved911 Jul 29, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it cover redirects like &>?

@preved911 preved911 Jul 29, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes — the well-known check looks at the target, not the operator. \u0026>/dev/null has target /dev/null → well-known. \u0026> output.txt has target output.txt → checked against edit rules. So &gt; redirects are fully covered.

if (redir.operator === "<<" || redir.operator === "<<-" || redir.operator === "<<<") return true;
return false;
}

function redirectToInfo(redir: Redirect): RedirectInfo {
return {
operator: redir.operator,
target: redir.target?.text ?? redir.content ?? "",
fileDescriptor: redir.fileDescriptor,
wellKnown: isWellKnownRedirect(redir),
};
}

function getCommandText(cmd: Command): string {
const parts: string[] = [];
if (cmd.name) {
Expand All @@ -21,6 +46,12 @@ function getCommandText(cmd: Command): string {
for (const word of cmd.suffix) {
parts.push(word.text);
}
for (const redir of cmd.redirects) {
const prefix = redir.fileDescriptor !== undefined ? String(redir.fileDescriptor) : "";
const op = redir.operator;
const target = redir.target?.text ?? "";
parts.push(`${prefix}${op}${target}`);
}
return parts.join(" ");
}

Expand Down Expand Up @@ -51,16 +82,23 @@ function extractCommandsFromNode(node: Node): Command[] {
return result;
}

function buildSegment(cmd: Command, stmtRedirects: Redirect[]): ChainSegment {
const cmdRedirects = (cmd.redirects ?? []).map(redirectToInfo);
const statementRedirects = (stmtRedirects ?? []).map(redirectToInfo);
return {
command: getCommandText(cmd),
commandName: getCommandName(cmd),
redirects: [...cmdRedirects, ...statementRedirects],
};
}

function extractCommandsFromScript(script: Script): ChainSegment[] {
const segments: ChainSegment[] = [];
for (const stmt of script.commands) {
const cmds = extractCommandsFromNode(stmt.command);
for (const cmd of cmds) {
if (cmd.type === "Command") {
segments.push({
command: getCommandText(cmd),
commandName: getCommandName(cmd),
});
segments.push(buildSegment(cmd, stmt.redirects));
}
}
}
Expand Down
Loading
Loading