From d8c615ac4c5b554175e7e98c7f383185d6feec02 Mon Sep 17 00:00:00 2001 From: Andrey Kumanyaev Date: Sat, 31 Oct 2026 11:40:23 +0100 Subject: [PATCH] parser: reject shell metacharacters in extractor-plugin file path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit extractor_plugin.go exports the repo-controlled file path into the child plugin environment as GORTEX_FILE_PATH=. The existing guard rejects control characters but leaves shell metacharacters, so a plugin that expands $GORTEX_FILE_PATH through a shell can be driven by a filename containing ; $ backticks | & ( ) < > quotes = or space — to inject commands, expand variables, splice env vars, or word-split the path. Reject shell metacharacters (shared shellUnsafePathChars set) in addition to control characters before exporting. The plugin command/args remain operator-declared config; only the repo-relative path is hardened. --- internal/parser/languages/extractor_plugin.go | 22 ++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/internal/parser/languages/extractor_plugin.go b/internal/parser/languages/extractor_plugin.go index 3fb8d4154..755e8fd41 100644 --- a/internal/parser/languages/extractor_plugin.go +++ b/internal/parser/languages/extractor_plugin.go @@ -433,6 +433,13 @@ func (e *SubprocessExtractor) Extract(filePath string, src []byte) (*parser.Extr return e.fileOnlyResult(filePath, src), nil } +// shellUnsafePathChars lists shell metacharacters that must not appear in +// GORTEX_FILE_PATH. A repo-controlled path that reaches a plugin which +// expands the variable through a shell could otherwise inject +// commands/operators or split the argument. The backtick is written as a +// raw-string literal; the `"` and `\` are escaped. +const shellUnsafePathChars = " ;|&<>()\"'\\$" + "`=" + // ExtractBounded runs the configured plugin with hard wire and result limits. // It is an all-or-nothing capability: caller cancellation, limit violations, // command failures, timeouts and malformed output return no partial result. @@ -450,11 +457,16 @@ func (e *SubprocessExtractor) ExtractBounded(ctx context.Context, filePath strin return nil, noUsage, &parser.ExtractionLimitError{Resource: "nodes", Limit: 0, Observed: 1} } // filePath is a repo-relative name the repository controls, so it can - // carry bytes that break env-var/shell handling in a plugin that later - // expands GORTEX_FILE_PATH. Reject control characters up front rather - // than exporting an unescaped path into the child environment. - if strings.ContainsAny(filePath, "\x00\n\r\t\v\f") { - return nil, noUsage, fmt.Errorf("extractor plugin: file path %q contains a control character", filePath) + // carry bytes that break env-var/shell handling when a plugin later + // expands $GORTEX_FILE_PATH. Reject control characters and shell + // metacharacters up front rather than exporting a value that, in a + // shell-expanding plugin, could inject commands (`;`), expand + // variables/commands (`$`, backticks), splice env vars (`=`), or + // word-split the path (space). `\\` and `` ` `` in the second set are + // the backslash and backtick; `\\$` is a literal dollar before the + // backtick (see the shellUnsafeChars const). + if strings.ContainsAny(filePath, "\x00\n\r\t\v\f") || strings.ContainsAny(filePath, shellUnsafePathChars) { + return nil, noUsage, fmt.Errorf("extractor plugin: file path %q contains a shell-unsafe character", filePath) } result := e.fileOnlyResult(filePath, src)