From 7d26fb32a717ce0b0f06e2573d54db615a1a6c88 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 3 May 2026 15:46:21 +0100 Subject: [PATCH] fix: preserve sudo shell carrier commands --- .../server-methods/server-methods.test.ts | 2 +- src/infra/command-analysis/explain.test.ts | 29 +++++++++++++++++-- src/infra/command-analysis/explain.ts | 24 +++++++-------- src/infra/command-analysis/risks.test.ts | 15 ++++++++++ src/infra/command-carriers.ts | 9 ++++++ 5 files changed, 63 insertions(+), 16 deletions(-) diff --git a/src/gateway/server-methods/server-methods.test.ts b/src/gateway/server-methods/server-methods.test.ts index e10262996d7..3cd08d2a012 100644 --- a/src/gateway/server-methods/server-methods.test.ts +++ b/src/gateway/server-methods/server-methods.test.ts @@ -908,7 +908,7 @@ describe("exec approval handlers", () => { twoPhase: true, host: "gateway", command: "python3 -c 'print(1)'", - commandArgv: ["python3", "-c", "print(1)"], + commandArgv: ["python3", "script.py"], systemRunPlan: undefined, nodeId: undefined, }, diff --git a/src/infra/command-analysis/explain.test.ts b/src/infra/command-analysis/explain.test.ts index 043e9ed6972..054f5a9cb97 100644 --- a/src/infra/command-analysis/explain.test.ts +++ b/src/infra/command-analysis/explain.test.ts @@ -31,11 +31,11 @@ describe("command-analysis explanation summary", () => { expect(summary.warningLines).toEqual(["Contains inline-eval: python3 -c"]); }); - it("resolves display summaries from argv or shell commands", () => { + it("resolves node display summaries from argv", () => { expect( resolveCommandAnalysisSummaryForDisplay({ - host: "gateway", - commandText: "echo ok", + host: "node", + commandText: "python3 script.py", commandArgv: ["python3", "-c", "print(1)"], }), ).toEqual( @@ -51,6 +51,29 @@ describe("command-analysis explanation summary", () => { commandText: "python3 -c 'print(1)'", }), ).toBeNull(); + }); + + it("resolves gateway display summaries from shell text even when argv is stale", () => { + expect( + resolveCommandAnalysisSummaryForDisplay({ + host: "gateway", + commandText: "python3 -c 'print(1)'", + commandArgv: ["python3", "script.py"], + }), + ).toEqual( + expect.objectContaining({ + commandCount: 1, + riskKinds: ["inline-eval"], + warningLines: ["Contains inline-eval: python3 -c"], + }), + ); + expect( + resolveCommandAnalysisSummaryForDisplay({ + host: "gateway", + commandText: "echo ok", + commandArgv: ["python3", "-c", "print(1)"], + })?.riskKinds, + ).toEqual([]); expect( resolveCommandAnalysisSummaryForDisplay({ host: "gateway", diff --git a/src/infra/command-analysis/explain.ts b/src/infra/command-analysis/explain.ts index 4d28d5a660b..0fbbed43e2c 100644 --- a/src/infra/command-analysis/explain.ts +++ b/src/infra/command-analysis/explain.ts @@ -89,19 +89,19 @@ export function resolveCommandAnalysisSummaryForDisplay(params: { sanitizeText?: (value: string) => string; }): CommandExplanationSummary | null { const analysis = - Array.isArray(params.commandArgv) && params.commandArgv.length > 0 - ? analyzeCommandForPolicy({ - source: "argv", - argv: params.commandArgv, - cwd: params.cwd ?? undefined, - }) - : params.host === "node" - ? null - : analyzeCommandForPolicy({ - source: "shell", - command: params.commandText, + params.host === "node" + ? Array.isArray(params.commandArgv) && params.commandArgv.length > 0 + ? analyzeCommandForPolicy({ + source: "argv", + argv: params.commandArgv, cwd: params.cwd ?? undefined, - }); + }) + : null + : analyzeCommandForPolicy({ + source: "shell", + command: params.commandText, + cwd: params.cwd ?? undefined, + }); if (!analysis?.ok) { return null; } diff --git a/src/infra/command-analysis/risks.test.ts b/src/infra/command-analysis/risks.test.ts index 748830100b4..46be84e1fea 100644 --- a/src/infra/command-analysis/risks.test.ts +++ b/src/infra/command-analysis/risks.test.ts @@ -18,6 +18,9 @@ describe("command-analysis risks", () => { ); expect(detectInlineEvalArgv(["sudo", "-uroot", "python3", "-c", "print(1)"])?.flag).toBe("-c"); expect(detectInlineEvalArgv(["sudo", "-EH", "python3", "-c", "print(1)"])?.flag).toBe("-c"); + expect(detectInlineEvalArgv(["sudo", "-i", "python3", "-c", "print(1)"])?.flag).toBe("-c"); + expect(detectInlineEvalArgv(["sudo", "-s", "python3", "-c", "print(1)"])?.flag).toBe("-c"); + expect(detectInlineEvalArgv(["sudo", "--shell", "python3", "-c", "print(1)"])?.flag).toBe("-c"); expect(detectInlineEvalArgv(["sudo", "-Eu", "root", "python3", "-c", "print(1)"])?.flag).toBe( "-c", ); @@ -106,6 +109,18 @@ describe("command-analysis risks", () => { expect(buildCommandPayloadCandidates(["sudo", "-EH", "/approve", "abc"])).toEqual([ "/approve abc", ]); + expect(buildCommandPayloadCandidates(["sudo", "-i", "/approve", "abc"])).toEqual([ + "/approve abc", + ]); + expect(buildCommandPayloadCandidates(["sudo", "-s", "/approve", "abc"])).toEqual([ + "/approve abc", + ]); + expect(buildCommandPayloadCandidates(["sudo", "--shell", "/approve", "abc"])).toEqual([ + "/approve abc", + ]); + expect(buildCommandPayloadCandidates(["sudo", "--preserve-groups", "/approve", "abc"])).toEqual( + ["/approve abc"], + ); expect( buildCommandPayloadCandidates(["sudo", "-uroot", "bash", "-lc", "/approve req allow-once"]), ).toEqual(["bash -lc /approve req allow-once", "/approve req allow-once"]); diff --git a/src/infra/command-carriers.ts b/src/infra/command-carriers.ts index 5e0da01c603..64c0bfe1a21 100644 --- a/src/infra/command-carriers.ts +++ b/src/infra/command-carriers.ts @@ -46,18 +46,27 @@ const SUDO_OPTIONS_WITH_VALUE = new Set([ ]); const SUDO_STANDALONE_OPTIONS = new Set([ "-A", + "-B", "-b", "-E", "-H", + "-i", + "-N", "-n", "-P", "-S", + "-s", "--askpass", "--background", + "--bell", "--login", + "--no-update", "--non-interactive", "--preserve-env", + "--preserve-groups", "--reset-home", + "--set-home", + "--shell", "--stdin", ]); const SUDO_NON_EXEC_OPTIONS = new Set([