fix(skills): stop reporting an env path, shebang or flag as an env dump (#6297)

* fix(skills): stop reporting an env path, shebang or flag as an env dump

* fix(skills): report shell env dumps only at a command position

`shell-env-dump` matched the word `env` anywhere it was not preceded by a path, dot, dash or word character, so ordinary argument text (`echo env`) and comment text (`# env`, `# export -p`) were still reported as environment dumps. Anchor the match to a shell command position -- the start of a line or right after a `;`, `&`, `|`, `(`, `)` or `{` separator, optionally behind `NAME=value` assignment prefixes -- and add regression cases for argument and comment text.

* fix(skills): ignore an env dump inside a comment or heredoc body

`shell-env-dump` matches `env`, `printenv` and `export -p` at a command
position, but the text it sees is still raw shell. A `;` inside a comment
(`# documentation; env is only an example`) and a bare `env` line inside
heredoc data both sit at a command position on paper while the shell runs
neither, so both were reported as environment dumps.

Reduce the text to shell code before matching: a quote-aware pass strips
comments (only a `#` at a word start opens one) and blanks heredoc bodies,
covering `<<-` tab stripping and quoted delimiters, while keeping one output
line per input line so reported line numbers stay correct. Real commands
after a separator (`true; env`) and on the line that opens a heredoc
(`env <<EOF`) still report.

* fix(skills): report env dumps after a keyword and stop masking ${#...}

The command-position anchor for `shell-env-dump` accepted only a line start or a
`;`, `&`, `|`, `(`, `)` or `{` separator, so real dumps were dropped: after a
reserved word that introduces a command (`if true; then env; fi`,
`while true; do printenv; done`), after `exec` (`exec env`) and inside a backtick
substitution (``V=`env` ``). Accept those positions too, with a guard so a path
component (`./then env`) does not arm the anchor, and add the backtick to the
separator set.

A `{` directly before a `#` is not a word start -- bash reads `${#HOME}` as the
length operator and `}#` as part of a word, while a brace group needs the space
of `{ # ...`. The comment heuristic no longer treats `{` and `}` as comment
openers, so `n=${#HOME}; env` keeps its `; env` and is reported again, while
`${env}` (a variable read, not a command) stays silent.

---------

Co-authored-by: sxh313 <sxh313@users.noreply.github.com>

backend/packages/harness/deerflow/skills/skillscan/orchestrator.py

@@ -145,6 +145,25 @@ _DESTRUCTIVE_RM_RE = (
     r"(?:-\S+\s+|--no-preserve-root\s+)*"
     r"/(?:\*|\s|$|(?:bin|boot|dev|etc|home|lib|lib64|opt|proc|root|run|sbin|srv|sys|usr|var)(?:/\*?)?(?:\s|$))"
 )
+# `env`, `printenv` and `export -p` dump the environment only when they run as a
+# command: at the start of a line, right after a `;`, `&`, `|`, `(`, `)` or
+# backtick separator, after the `{` that opens a brace group (`{ env; }`, which
+# bash requires to be followed by whitespace), or after a reserved word that must
+# introduce a command (`then`, `do`, `exec`, ...). An `NAME=value` assignment
+# prefix may come first. Anywhere else the word names something else -- a path in
+# `#!/usr/bin/env bash`, a host in `https://env.example.com`, a flag in
+# `--env FOO=1`, an argument in `echo env`, a variable in `${env}`, or comment
+# text in `# export -p` -- and dumps nothing. The text this is matched against is
+# first reduced to shell code by `_shell_code_only`, so a `;` inside a comment and
+# a command-looking line inside a heredoc body do not count either.
+_SHELL_ENV_DUMP_RE = re.compile(
+    r"(?m)(?:^|(?<=[;&|()`])|\{(?=[ \t])|(?<![\w/.-])(?:if|then|elif|else|while|until|do|exec)\b[ \t]+)"
+    r"[ \t]*(?:[A-Za-z_]\w*=[^ \t]*[ \t]+)*(?P<cmd>env\b|printenv\b|export[ \t]+-p\b)"
+)
+# The head of a heredoc redirection: `<<` or `<<-`, an optional quoted delimiter,
+# then the delimiter word. Requiring a leading letter/underscore keeps arithmetic
+# shifts such as `$((1 << 2))` from being read as a heredoc opener.
+_HEREDOC_HEAD_RE = re.compile(r"<<(-?)[ \t]*([\"']?)([A-Za-z_]\w*)\2")
 
 
 def skill_scan_enabled(app_config: Any | None = None) -> bool:
@@ -692,6 +711,72 @@ def _scan_python(rel_path: str, text: str) -> list[SecurityFinding]:
     return findings
 
 
+def _split_shell_line(line: str) -> tuple[str, list[tuple[str, bool]]]:
+    """Split one shell line into its code part and the heredocs it declares.
+
+    Quote-aware: a `#` outside quotes starts a comment only at a word start, and
+    `<<` outside quotes opens a heredoc. Returns the code text (comment stripped)
+    together with the `(delimiter, strip_tabs)` pairs the line opens.
+
+    A `{` or `}` immediately before the `#` is not a word start: bash reads
+    `${#HOME}` as the length operator and `}#` as part of a word, while a brace
+    group needs the space of `{ # ...`. Those two characters are therefore left
+    out of the set that admits a comment.
+    """
+    heredocs: list[tuple[str, bool]] = []
+    in_single = in_double = False
+    i = 0
+    while i < len(line):
+        ch = line[i]
+        if in_single:
+            in_single = ch != "'"
+        elif in_double:
+            if ch == "\\":
+                i += 2
+                continue
+            in_double = ch != '"'
+        elif ch == "\\":
+            i += 2
+            continue
+        elif ch == "'":
+            in_single = True
+        elif ch == '"':
+            in_double = True
+        elif ch == "#" and (i == 0 or line[i - 1] in " \t;&|()<>"):
+            return line[:i], heredocs
+        elif ch == "<" and line.startswith("<<", i) and not line.startswith("<<<", i) and (i == 0 or not (line[i - 1].isalnum() or line[i - 1] == "_")):
+            if head := _HEREDOC_HEAD_RE.match(line, i):
+                heredocs.append((head.group(3), head.group(1) == "-"))
+                i = head.end()
+                continue
+        i += 1
+    return line, heredocs
+
+
+def _shell_code_only(text: str) -> str:
+    """Blank out comment text and heredoc bodies, preserving line structure.
+
+    `_SHELL_ENV_DUMP_RE` matches at a command position, but a `;` inside a
+    comment (`# documentation; env is only an example`) and a bare `env` line
+    inside heredoc data are not commands. Replacing them with blanks -- one
+    output line per input line, so `_line_number` stays correct -- leaves the
+    matcher looking only at shell code.
+    """
+    lines: list[str] = []
+    pending: list[tuple[str, bool]] = []
+    for raw in text.split("\n"):
+        if pending:
+            delimiter, strip_tabs = pending[0]
+            if (raw.lstrip("\t") if strip_tabs else raw) == delimiter:
+                pending.pop(0)
+            lines.append("")
+            continue
+        code, heredocs = _split_shell_line(raw)
+        pending.extend(heredocs)
+        lines.append(code)
+    return "\n".join(lines)
+
+
 def _scan_shell(rel_path: str, text: str) -> list[SecurityFinding]:
     findings: list[SecurityFinding] = []
     # Unmistakable reverse-shell signals hard-block; weaker idioms (bash -i,
@@ -706,8 +791,10 @@ def _scan_shell(rel_path: str, text: str) -> list[SecurityFinding]:
         findings.append(_finding_from_match("shell-curl-pipe-shell", rel_path, text, match))
     if match := re.search(_DESTRUCTIVE_RM_RE + r"|:\(\)\{\s*:\|:&\s*\};:|dd\s+[^#\n]*\bof=/dev/", text):
         findings.append(_finding_from_match("shell-destructive-command", rel_path, text, match))
-    if match := re.search(r"\b(env|printenv|export\s+-p)\b", text):
-        findings.append(_finding_from_match("shell-env-dump", rel_path, text, match))
+    # Only a command position counts, and only in shell code: see `_shell_code_only`.
+    code = _shell_code_only(text)
+    if match := _SHELL_ENV_DUMP_RE.search(code):
+        findings.append(_finding("shell-env-dump", file=rel_path, line=_line_number(code, match.start("cmd")), evidence=match.group("cmd")))
     return findings
 
 

backend/tests/test_skillscan_shell_env_dump.py

@@ -0,0 +1,208 @@
+"""`shell-env-dump` must report an environment dump, not a path or a shebang.
+
+The rule's own message is "Shell script dumps the environment", so only a command
+that actually dumps it counts. The word `env` also appears as a path component in
+the portable shebang (`#!/usr/bin/env bash`, used by this repository's own
+`skills/public/claude-to-deerflow/scripts/*.sh`), in host labels
+(`https://env.example.com`), in dotted file names and in option names
+(`--env`), as a plain command argument (`echo env`), as a variable (`${env}`) and
+inside comments (`# export -p`); none of them dumps anything, so the command word
+must sit at the start of a command: a line start, a position after a `;`, `&`,
+`|`, `(`, `)` or backtick separator, the `{` of a brace group, or the position
+after a reserved word that introduces a command (`then`, `do`, `exec`, ...).
+Comment text after a separator and heredoc bodies are data rather than commands
+too, so they are stripped before that command-position match.
+"""
+
+from __future__ import annotations
+
+from pathlib import Path
+
+from deerflow.skills.skillscan.orchestrator import scan_skill_dir
+
+
+def _write_script(skill_dir: Path, body: str) -> None:
+    scripts = skill_dir / "scripts"
+    scripts.mkdir(parents=True, exist_ok=True)
+    (skill_dir / "SKILL.md").write_text("---\nname: demo\ndescription: A demo skill\n---\n\nBody.\n", encoding="utf-8")
+    (scripts / "run.sh").write_text(body, encoding="utf-8")
+
+
+def _rules(skill_dir: Path) -> set[str]:
+    return {finding["rule_id"] for finding in scan_skill_dir(skill_dir)["findings"]}
+
+
+def test_env_shebang_is_not_an_environment_dump(tmp_path: Path) -> None:
+    """The portable shebang names a path, so it is not an `env` invocation."""
+    _write_script(tmp_path, "#!/usr/bin/env bash\necho hello\n")
+    assert "shell-env-dump" not in _rules(tmp_path)
+
+
+def test_env_in_a_url_host_is_not_an_environment_dump(tmp_path: Path) -> None:
+    """A host label that happens to read `env` is not an environment dump."""
+    _write_script(tmp_path, "#!/bin/bash\ncurl https://env.example.com/x\n")
+    assert "shell-env-dump" not in _rules(tmp_path)
+
+
+def test_env_file_name_is_not_an_environment_dump(tmp_path: Path) -> None:
+    """`source .env` loads one file; it does not dump the environment."""
+    _write_script(tmp_path, "#!/bin/bash\nset -a\nsource .env\n")
+    assert "shell-env-dump" not in _rules(tmp_path)
+
+
+def test_env_option_name_is_not_an_environment_dump(tmp_path: Path) -> None:
+    """`--env MODE=fast` sets one variable for one command; it is not a dump."""
+    _write_script(tmp_path, "#!/bin/bash\ndocker run --env MODE=fast image\n")
+    assert "shell-env-dump" not in _rules(tmp_path)
+
+
+def test_bare_env_command_still_reports(tmp_path: Path) -> None:
+    """A bare `env` is exactly the dump the rule exists for."""
+    _write_script(tmp_path, "#!/bin/bash\nenv\n")
+    assert "shell-env-dump" in _rules(tmp_path)
+
+
+def test_printenv_command_still_reports(tmp_path: Path) -> None:
+    _write_script(tmp_path, "#!/bin/bash\nprintenv | sort\n")
+    assert "shell-env-dump" in _rules(tmp_path)
+
+
+def test_export_p_still_reports(tmp_path: Path) -> None:
+    _write_script(tmp_path, "#!/bin/bash\nexport -p\n")
+    assert "shell-env-dump" in _rules(tmp_path)
+
+
+def test_env_as_a_command_argument_is_not_an_environment_dump(tmp_path: Path) -> None:
+    """`echo env` prints the word, not the environment: `env` is an argument here."""
+    _write_script(tmp_path, "#!/bin/bash\necho env\n")
+    assert "shell-env-dump" not in _rules(tmp_path)
+
+
+def test_env_in_a_comment_is_not_an_environment_dump(tmp_path: Path) -> None:
+    """Comment text is not a command, so `# env` dumps nothing."""
+    _write_script(tmp_path, "#!/bin/bash\n# env\n")
+    assert "shell-env-dump" not in _rules(tmp_path)
+
+
+def test_export_p_in_a_comment_is_not_an_environment_dump(tmp_path: Path) -> None:
+    """A commented `export -p` is documentation, not an invocation."""
+    _write_script(tmp_path, "#!/bin/bash\n# export -p prints the environment\n")
+    assert "shell-env-dump" not in _rules(tmp_path)
+
+
+def test_env_after_a_command_separator_still_reports(tmp_path: Path) -> None:
+    """`true; env` runs `env` as a second command on the line."""
+    _write_script(tmp_path, "#!/bin/bash\ntrue; env\n")
+    assert "shell-env-dump" in _rules(tmp_path)
+
+
+def test_env_after_a_pipe_still_reports(tmp_path: Path) -> None:
+    _write_script(tmp_path, "#!/bin/bash\ncat file | printenv\n")
+    assert "shell-env-dump" in _rules(tmp_path)
+
+
+def test_indented_env_still_reports(tmp_path: Path) -> None:
+    """Indentation is not a command argument, so a line-leading `env` still dumps."""
+    _write_script(tmp_path, "#!/bin/bash\n    env\n")
+    assert "shell-env-dump" in _rules(tmp_path)
+
+
+def test_env_after_an_assignment_prefix_still_reports(tmp_path: Path) -> None:
+    """`FOO=1 env` runs `env`; the assignment is a prefix, not an argument."""
+    _write_script(tmp_path, "#!/bin/bash\nFOO=1 env\n")
+    assert "shell-env-dump" in _rules(tmp_path)
+
+
+def test_env_after_a_separator_inside_a_comment_is_not_an_environment_dump(tmp_path: Path) -> None:
+    """A `;` inside a comment does not turn its text into a second command."""
+    _write_script(tmp_path, "#!/bin/bash\n# documentation; env is only an example\n")
+    assert "shell-env-dump" not in _rules(tmp_path)
+
+
+def test_export_p_after_a_separator_inside_a_comment_is_not_an_environment_dump(tmp_path: Path) -> None:
+    """Comment text after a separator is documentation, not an invocation."""
+    _write_script(tmp_path, "#!/bin/bash\necho hi # ; export -p\n")
+    assert "shell-env-dump" not in _rules(tmp_path)
+
+
+def test_env_in_a_quoted_heredoc_body_is_not_an_environment_dump(tmp_path: Path) -> None:
+    """A line inside heredoc data is input, not a command."""
+    _write_script(tmp_path, "#!/bin/bash\ncat <<'EOF'\nenv\nEOF\n")
+    assert "shell-env-dump" not in _rules(tmp_path)
+
+
+def test_printenv_in_a_tab_stripped_heredoc_body_is_not_an_environment_dump(tmp_path: Path) -> None:
+    """`<<-` strips leading tabs from the body and its terminator alike."""
+    _write_script(tmp_path, "#!/bin/bash\ncat <<-EOF\n\tprintenv\n\tEOF\n")
+    assert "shell-env-dump" not in _rules(tmp_path)
+
+
+def test_env_on_the_line_that_opens_a_heredoc_still_reports(tmp_path: Path) -> None:
+    """Only the heredoc body is data; the command before it is still code."""
+    _write_script(tmp_path, "#!/bin/bash\nenv <<EOF\nFOO\nEOF\n")
+    assert "shell-env-dump" in _rules(tmp_path)
+
+
+def test_export_p_after_a_command_separator_still_reports(tmp_path: Path) -> None:
+    """`echo hi; export -p` runs `export -p` as a second command on the line."""
+    _write_script(tmp_path, "#!/bin/bash\necho hi; export -p\n")
+    assert "shell-env-dump" in _rules(tmp_path)
+
+
+def test_env_after_then_still_reports(tmp_path: Path) -> None:
+    """`then` introduces a command list, so an `env` there is a real invocation."""
+    _write_script(tmp_path, "#!/bin/bash\nif true; then env; fi\n")
+    assert "shell-env-dump" in _rules(tmp_path)
+
+
+def test_printenv_after_do_still_reports(tmp_path: Path) -> None:
+    """A loop body is a command list too, so `do printenv` dumps the environment."""
+    _write_script(tmp_path, "#!/bin/bash\nwhile true; do printenv; done\n")
+    assert "shell-env-dump" in _rules(tmp_path)
+
+
+def test_env_as_a_condition_still_reports(tmp_path: Path) -> None:
+    """`if env; then ...` runs `env` as the condition command."""
+    _write_script(tmp_path, "#!/bin/bash\nif env; then :; fi\n")
+    assert "shell-env-dump" in _rules(tmp_path)
+
+
+def test_env_after_exec_still_reports(tmp_path: Path) -> None:
+    _write_script(tmp_path, "#!/bin/bash\nexec env\n")
+    assert "shell-env-dump" in _rules(tmp_path)
+
+
+def test_env_in_a_backtick_substitution_still_reports(tmp_path: Path) -> None:
+    """A backtick opens a command substitution, just as `$(` does."""
+    _write_script(tmp_path, "#!/bin/bash\nV=`env`\n")
+    assert "shell-env-dump" in _rules(tmp_path)
+
+
+def test_env_in_a_brace_group_still_reports(tmp_path: Path) -> None:
+    """A brace group is written `{ env; }`, so the `{` is followed by whitespace."""
+    _write_script(tmp_path, "#!/bin/bash\n{ env; }\n")
+    assert "shell-env-dump" in _rules(tmp_path)
+
+
+def test_env_in_a_parameter_expansion_is_not_an_environment_dump(tmp_path: Path) -> None:
+    """`${env}` reads one variable; that `{` does not open a command group."""
+    _write_script(tmp_path, "#!/bin/bash\nx=${env}\n")
+    assert "shell-env-dump" not in _rules(tmp_path)
+
+
+def test_keyword_like_path_component_is_not_a_command_position(tmp_path: Path) -> None:
+    """`./then env` runs a local program named `then`; `env` is its argument."""
+    _write_script(tmp_path, "#!/bin/bash\n./then env\n")
+    assert "shell-env-dump" not in _rules(tmp_path)
+
+
+def test_env_after_a_length_expansion_still_reports(tmp_path: Path) -> None:
+    """`${#HOME}` is not a comment, so the dump after the `;` is still real code."""
+    _write_script(tmp_path, "#!/bin/bash\nn=${#HOME}; env\n")
+    assert "shell-env-dump" in _rules(tmp_path)
+
+
+def test_printenv_after_an_array_length_expansion_still_reports(tmp_path: Path) -> None:
+    """`${#arr[@]}` keeps the line as code, so its `;` starts a real command."""
+    _write_script(tmp_path, "#!/bin/bash\nn=${#arr[@]}; printenv\n")
+    assert "shell-env-dump" in _rules(tmp_path)