Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/system-specs/modules/security.md
Original file line number Diff line number Diff line change
Expand Up @@ -1739,6 +1739,7 @@ continue to apply.
- `_PUSH_ALL_BRANCHES_OPTS` covers `--mirror`, `--all`, and Git's `--branches` alias are denied **outright** (they push every local branch, so a per-branch target check cannot vouch for them), kept in lockstep with the `--(mirror|all)` regex in `config/defaults.json`.
- `_is_push_to_protected_branch()` splits the command with `_split_segments()` and validates **every** `push` segment / refspec (closing the `push origin feat && push origin main` bypass), normalizing `refs/heads/…` paths and `local:remote` refspecs; refspecs with shell/revision syntax (`$`, `` ` ``, `@{…}` — `_AMBIGUOUS_REFSPEC_RE`) are treated as ambiguous and denied. If a push was detected upstream but **no** clean segment parses, it denies to be safe.
- **Force push:** a force flag (`--force` / `-f` / `--force-with-lease`) does not by itself make a feature-branch push protected (force-push to a feature branch is normal PR/rebase workflow), but force-push to a *protected* branch is still blocked because the target check fires regardless of flags.
- **A push line inside a literal `gh` text value is data.** The segment split ends at every newline, quoted or not, so on its own it reads the middle line of `gh pr create --body "notes⏎git push⏎more"` as a bare push. `shell_normalizer_walk._gh_data_newlines_as_spaces` reads that one shape as its single-line spelling: the whole command is one `gh pr|issue|release …` of plain words (`[A-Za-z0-9._/:=+,-]`, so no PowerShell `--%` stop-parsing token or `@` splat) and whole quoted words, every quote closes and stands between blanks, no quoted word holds a curly quote mark (PowerShell reads `‘ ’ ‚ ‛ “ ” „` as quotes too), a double-quoted word holds no `$`, backtick or backslash, and each multi-line quoted word is the value of `-b/--body`, `-t/--title`, `--subject` or `-n/--notes`. With no separator, comment, heredoc, escape or expansion outside the quotes, bash's and PowerShell's quote reading of that text is the walk's, and gh is the only program the shell runs (gh refuses an alias or extension named after a core command). `is_denied` reads that view before it hands any source to `_git_publish_floor_tags`, so such a command is no publish and records no `push_allowed` audit. That reading is withheld where the source is not the script the shell runs: any nested source the payload walk yields (`source != lower` in `deny_gate.is_denied`), since a filter such as `tr -d '"'` or `sed` between it and its evaluator may rewrite it first, a source with an unresolved printf conversion (`%s`, `%b`, ...; `%%` is a literal `%`), whose arguments can close the body before the push line, and any command holding a backslash at all (`undecoded` in `deny_gate.is_denied`), since every escape decoder can make a quote bash never prints: `str.lower` turns `\X27`, which printf and `echo -e` print as text, into a decoded quote, a printf format reads `\0047` as `\004` + `7`, and bash's `echo` prints `\47` as text. Any other text is returned unchanged and keeps its per-line reading: an env prefix, `cd … &&`, a pipe or redirect, another owner (`bash -c`, `sh -c`, `eval`, `echo`, `git commit -m`), a positional or non-text-flag value, and every quote trick (comment apostrophe, heredoc, backslash-newline, unbalanced or glued quote). kiro-cli titles a shell call `Running: <command>` and the deny tiers judge that title as a subject of its own, so one exact leading `running: ` (`shell_normalizer_walk._SHELL_TITLE_PREFIX`, the lowercased view; `denied_rules._exception_applies` reads through the same constant) is read through and kept: the title gets its command's reading. A second prefix, any other spelling, or one that does not start the text is read as text, so a title never gets a weaker verdict than the command it displays. On Windows the text is always returned unchanged (`_QUOTED_NEWLINE_MAY_END_COMMAND`): cmd.exe ends a command at a newline even inside double quotes, and nothing proves which shell runs an agent's command there. Pinned by `test_push_branch_gate.py::TestQuotedGhTextValueIsNotAPush`.
- **Self-protection (argv-structural floor):** the controls that stop the agent disabling its own controls — credential minting, process termination, the dev-mode attestation flag, the ssh-family sandbox escape, and the `restart` / `update` / `cloud` / `gateway restart` lifecycle commands, the `file-delivery approve` consent step-up, and the `skills authority-retire` recovery — are enforced by dedicated argv predicates. They come in two shapes, and the difference is whether a catalog row exists to gate on:
- **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; the table is keyed on the file's stat plus, for a file up to 64 KiB, a content digest on every platform, since a same-size rewrite inside one coarse ctime tick leaves the stat unchanged (a larger POSIX file keeps a stat-only key, so a same-size rewrite inside the ctime tick of its previous write is still served stale); 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-file-delivery`, `self-protection-gateway-restart`, `self-protection-cloud`, `self-protection-skills-authority-retire`). These six have **no catalog row**: the four lifecycle floors' regex rows fired on the product's name appearing anywhere in a command and were deleted (see "Self-protection global options" above), the `file-delivery approve` floor never had one: it denies completing a flagged-file delivery consent step-up (the read-only `file-delivery` and `file-delivery --help` forms stay allowed), and the `skills authority-retire` floor never had one either: it denies the operator-terminal recovery that moves the auto-skill publication authority aside (`kirocrew skills authority-retire`), which an agent shell must not run because the authority is what keeps an agent from loosening its own skill governance. Like the other subcommand floors, it refuses that command's `--help` form too; the refusal note sends the operator to their own terminal, and `kirocrew skills --help` stays allowed. 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.
Expand Down
6 changes: 1 addition & 5 deletions src/kiro_crew/security/denied_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
BUILTIN_DENY_PATTERNS,
)
from .diagnostics import RefusalDiagnostic, annotate_refusal, refusal_diagnostic
from .shell_normalizer import _SHELL_ACTIVE_CHARS
from .shell_normalizer import _SHELL_ACTIVE_CHARS, _SHELL_TITLE_PREFIX

if TYPE_CHECKING:
from collections.abc import Iterable
Expand Down Expand Up @@ -488,10 +488,6 @@ def _exception_eligible(view: str) -> bool:
return not _SHELL_ACTIVE_CHARS.intersection(view)


#: kiro-cli's display prefix on a shell call's title, lowercased like the views.
_SHELL_TITLE_PREFIX = "running: "


def _exception_applies(view: str, exceptions: list[str]) -> bool:
"""Whether one of *exceptions* exonerates the lowercased *view*.

Expand Down
34 changes: 33 additions & 1 deletion src/kiro_crew/security/deny_gate.py
Original file line number Diff line number Diff line change
Expand Up @@ -622,12 +622,44 @@ def _reason(
# the nested coverage and nothing else. Failing closed here instead
# would refuse ordinary commands on any walk hiccup.
payload_sources = [lower]
# The walk printf-decodes each payload, which can make a quote bash never
# sees (``\x27`` is ``x27`` to bash), so the bash-read sources are judged too.
if "\\" in lower:
try:
for source, _tokens in _argv._shell_payload_walk(lower, printf_decode=False):
if source not in payload_sources:
payload_sources.append(source)
except Exception:
# Same no-raise contract as above; the decoded sources still stand.
pass
# Tags are collected across EVERY publish source, not just the top-level
# string, and gated once afterwards. Reading only ``lower`` made one wrapper a
# complete bypass of the sole enforcement pushes have; gating once at the end
# keeps the per-rule opt-out semantics exactly as written -- a rule an operator
# disabled stays disabled at whatever depth it fires.
publish_sources = [source for source in payload_sources if _argv._is_git_publish(source)]
# A source whose every push line lies inside a literal gh text value is not
# a publish, so the floor never reads it and it records no ``push_allowed``
# audit either. That reading holds only where the source IS the script the
# shell runs, so a source keeps the per-line reading whenever the command
# holds a backslash or a printf conversion. Every escape decoder here can
# make a quote the shell never sees: lowercasing turns ``\X27`` (text) into
# ``\x27`` (a quote), a printf FORMAT reads ``\0047`` as ``\004`` + ``7``,
# and bash's echo prints ``\47`` as text. A conversion its arguments fill
# (``%s`` -> ``"``) can close the body before the push line; ``%%`` is a
# literal ``%``. A command with neither is decoded by nothing. A NESTED
# source keeps the per-line reading too: a filter such as ``tr`` or ``sed``
# may rewrite its text (drop the quotes) before the evaluator reads it.
_spaced = _submodule("shell_normalizer")._gh_data_newlines_as_spaces
undecoded = "\\" not in tool_name
publish_sources = [
source
for source in payload_sources
if _argv._is_git_publish(
_spaced(source)
if source == lower and undecoded and "%" not in source.replace("%%", "")
else source
)
]
if publish_sources:
floor_tags: frozenset[str] = frozenset()
for publish_source in publish_sources:
Expand Down
35 changes: 31 additions & 4 deletions src/kiro_crew/security/shell_normalizer.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@
_PROCESS_SUBSTITUTION_OPENERS,
_SHELL_ACTIVE_CHARS,
_SHELL_ASSIGN_RE,
_SHELL_TITLE_PREFIX,
_SHELL_WRAPPER_CHARS,
_VAR_USE_RE,
_arms_case_context,
Expand All @@ -90,6 +91,7 @@
_dequote_token,
_ends_argv,
_fold_line_continuations,
_gh_data_newlines_as_spaces,
_is_computed_value,
_is_shell_variable_reference,
_iter_shell_chars,
Expand Down Expand Up @@ -1133,6 +1135,7 @@ def _nested_shell_payloads(
*,
allow_join: bool = True,
joined_out: "set[str] | None" = None,
printf_decode: bool = True,
) -> "list[str]":
"""Literal shell-script payloads carried as an argument inside *tokens*.

Expand All @@ -1155,6 +1158,9 @@ def _nested_shell_payloads(
origin main'`` is fused to ``git push origin main`` by the first join, so the
publish is visible at the first joined frame and the chain would only
re-derive suffixes of an answer already in hand.

*printf_decode* False makes the evaluator-pipe branch yield the producer
token as bash reads it; see :func:`_shell_payload_walk`.
"""
payloads: list[str] = []
# Both scans below look for the FIRST token after a program that satisfies a stop
Expand Down Expand Up @@ -1396,9 +1402,22 @@ def _first_stop_at_or_after(stops: "list[int]", start: int) -> int:
# so the "is this a command line?" test is applied to the DECODED text -- otherwise
# the token still looks like a single word and is never recognised as a payload at
# all. Splitting on any whitespace (not just a space) also admits a tab escape.
# The bash-read walk (*printf_decode* False) yields the producer token
# UNDECODED: ``echo`` / ``printf '%s'`` print ``\\x27`` as four characters,
# which the shell reads as ``x27``, not a quote. A ``printf`` FORMAT decodes
# exactly ONCE, so that walk also yields the once-decoded text and never
# decodes it again (``\\x5cx27`` prints ``\\x27``, still ``x27`` to bash).
# The multi-word test still reads the decoded text, so this walk admits
# every token the decoded one does.
for token in tokens:
decoded = _decode_printf_escapes(token)
if len(decoded.split()) > 1:
if printf_decode:
if len(decoded.split()) > 1:
payloads.append(decoded)
continue
if len(token.split()) > 1:
payloads.append(token)
if decoded != token and len(decoded.split()) > 1:
payloads.append(decoded)
# ``xargs`` is different in shape: it does not read a whole script, it APPENDS
# the piped words to its own command. ``echo <verb> | xargs <name>`` therefore
Expand All @@ -1413,9 +1432,14 @@ def _first_stop_at_or_after(stops: "list[int]", start: int) -> int:
# ``${a[@]}`` / ``${a[*]}`` / ``$a[@]`` -- the whole array as separate words.


def _shell_payload_walk(text_lower: str) -> "list[tuple[str, list[str]]]":
def _shell_payload_walk(
text_lower: str, printf_decode: bool = True
) -> "list[tuple[str, list[str]]]":
"""``(source, argv)`` for *text_lower* and every nested shell payload in it.

*printf_decode* False keeps each payload as bash reads it, with no printf
escape decoded (an unquoted ``\\x27`` is ``x27`` to bash, not a quote).

``bash -c "kirocrew token"`` tokenizes to ``['bash', '-c', 'kirocrew token']``
-- the dangerous command is a single opaque token, so a direct scan cannot
see it. Re-tokenizing the payload and checking that view too closes the
Expand Down Expand Up @@ -1465,12 +1489,15 @@ class rather than one spelling of it.
# reads folded, and the fold preserves single-quoted and ANSI-C spans, so a
# continuation that bash keeps literal stays literal in the body too.
joined_here: set[str] = set()
nested = _nested_shell_payloads(tokens, allow_join=allow_join, joined_out=joined_here)
nested = _nested_shell_payloads(
tokens, allow_join=allow_join, joined_out=joined_here, printf_decode=printf_decode
)
for payload in list(nested) + _substitution_bodies(_fold_line_continuations(source)):
# Descend through EVERY literal payload, to any depth. Termination is
# structural, not a cap: a payload is carried inside one token of its
# parent, so it is strictly shorter than the parent's source text.
payload = _decode_printf_escapes(payload)
if printf_decode:
payload = _decode_printf_escapes(payload)
if len(payload) >= parent_len or payload in seen:
continue
seen.add(payload)
Expand Down
Loading
Loading