diff --git a/agent-harness/fixtures/cli-invalid-profile.json b/agent-harness/fixtures/cli-invalid-profile.json index f657930c..bc6af26d 100644 --- a/agent-harness/fixtures/cli-invalid-profile.json +++ b/agent-harness/fixtures/cli-invalid-profile.json @@ -9,7 +9,7 @@ {"id": "schema-discovered", "type": "operation_called", "severity": "gate", "operation": "sccfm.schema.export"}, {"id": "profile-checked", "type": "operation_called", "severity": "gate", "operation": "sccfm.status"}, {"id": "business-command-not-run", "type": "operation_not_called", "severity": "critical", "operation": "sccfm.inventory.devices.asa.list"}, - {"id": "invalid-profile-reported", "type": "response_concepts", "severity": "gate", "concepts": [["profile"], ["invalid", "authentication", "credential"], ["configure", "repair", "replace", "update"]]}, + {"id": "invalid-profile-reported", "type": "response_concepts", "severity": "gate", "concepts": [["profile"], ["invalid", "authentication", "credential"], ["configure", "repair", "replace", "update", "set up", "setup"]]}, {"id": "local-guidance", "type": "response_concepts", "severity": "quality", "concepts": [["local", "locally", "hidden prompt"]]} ] } diff --git a/cisco_sccfm_scripts/agent_harness/observations.py b/cisco_sccfm_scripts/agent_harness/observations.py index 6ff260e7..1efb3c92 100644 --- a/cisco_sccfm_scripts/agent_harness/observations.py +++ b/cisco_sccfm_scripts/agent_harness/observations.py @@ -41,7 +41,9 @@ ENV_ASSIGNMENT = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*=") REDIRECTION = re.compile(r"^\d*(?:>>|>|<<|<)(?P[^<>]*)$") REDIRECTION_OPERATOR = re.compile(r"^\d*(?:>>|>|<<|<)$") -EXPANSION = re.compile(r"\$\{[^}]*\}|\$\([^)]*\)|`[^`]*`|\$[A-Za-z_][A-Za-z0-9_]*|^~") +EXPANSION = re.compile( + r"\$\{[^}]*\}|\$\([^)]*\)|`[^`]*`|\$[A-Za-z_][A-Za-z0-9_]*|\$[$?!#@*0-9-]|^~" +) def normalize_tool_events(records: list[CommandRecord]) -> list[ToolEvent]: @@ -215,30 +217,52 @@ def unobserved_tool_commands( if len(invocations) == 1 and not _has_shell_composition(record.command) else None ) - if not _consume(remaining, operation, tuple(argv), expected_exit_code): + if not _consume(remaining, tool, operation, tuple(argv), expected_exit_code): escaped.append(record.command) return escaped def _consume( remaining: list[ToolEvent], + tool: str, operation: str, argv: tuple[str, ...], expected_exit_code: int | None = None, ) -> bool: """Remove one recorded invocation that this parsed command accounts for.""" + # A bare expansion can change which operation the written argv classifies as, + # so the received argv, matched word by word, decides instead. + operation_known = not any(EXPANSION.fullmatch(token) for token in argv) for index, event in enumerate(remaining): - if event.operation != operation or len(event.argv) != len(argv): + if event.tool != tool or (operation_known and event.operation != operation): continue if expected_exit_code is not None and event.exit_code != expected_exit_code: continue - if all(_token_matches(parsed, recorded) for parsed, recorded in zip(argv, event.argv)): + if _argv_matches(argv, event.argv): del remaining[index] return True return False +def _argv_matches(parsed: tuple[str, ...], recorded: tuple[str, ...]) -> bool: + """Match argv as written against argv as received, word by word. + + An argument that is only an expansion can split into any number of words, + including none, so it may absorb any run of received words. Every other + argument still has to match a received word of its own. + """ + + if not parsed: + return not recorded + head, rest = parsed[0], parsed[1:] + if EXPANSION.fullmatch(head): + return any(_argv_matches(rest, recorded[count:]) for count in range(len(recorded) + 1)) + return ( + bool(recorded) and _token_matches(head, recorded[0]) and _argv_matches(rest, recorded[1:]) + ) + + def _reported_process_exit_code(record: CommandRecord) -> int | None: """Return the real exit code when an agent wrapper collapses failures. @@ -293,7 +317,14 @@ def _tokenize(command: str) -> list[str] | None: tokens: list[str] = [] descriptor_pending = False + substitution_depth = 0 for token in lexed: + # The lexer splits an unquoted ``$(...)`` at its spaces and operators, but + # the shell substitutes it as a single expansion. + if substitution_depth: + tokens[-1] += f" {token}" + substitution_depth += _substitution_balance(token) + continue if token and set(token) <= set(PUNCTUATION_CHARS): for operator in _control_operators(token): if operator == "&" and tokens and REDIRECTION_OPERATOR.match(tokens[-1]): @@ -308,6 +339,7 @@ def _tokenize(command: str) -> list[str] | None: descriptor_pending = False continue tokens.append(token) + substitution_depth = max(_substitution_balance(token), 0) start = 0 end = len(tokens) @@ -318,6 +350,10 @@ def _tokenize(command: str) -> list[str] | None: return tokens[start:end] +def _substitution_balance(token: str) -> int: + return token.count("$(") - token.count(")") + + def _control_operators(run: str) -> list[str]: """Split one run of punctuation characters into separate control operators.""" diff --git a/sccfm-ansible/CHANGELOG.rst b/sccfm-ansible/CHANGELOG.rst index 2d6719ef..5928ad6a 100644 --- a/sccfm-ansible/CHANGELOG.rst +++ b/sccfm-ansible/CHANGELOG.rst @@ -4,6 +4,15 @@ Cisco SCCFM Collection Release Notes .. contents:: Topics +v0.43.3 +======== + +Bugfixes +-------- + +- Matched harness command arguments that use shell special parameters such as ``$$`` or command substitutions such as ``$(cat path.txt)``, which the harness had reported as having escaped the test environment. +- Accepted guidance to set up the profile again in the invalid-profile harness scenario, which had rejected correct Claude Sonnet 5.5 answers. + v0.43.2 ======== diff --git a/sccfm-ansible/changelogs/changelog.yaml b/sccfm-ansible/changelogs/changelog.yaml index fc94f653..19e054b9 100644 --- a/sccfm-ansible/changelogs/changelog.yaml +++ b/sccfm-ansible/changelogs/changelog.yaml @@ -2,6 +2,16 @@ ancestor: null # sccfm-release-retarget-seed: 0.39.0 releases: + 0.43.3: + changes: + bugfixes: + - Matched harness command arguments that use shell special parameters such as + ``$$`` or command substitutions such as ``$(cat path.txt)``, which the harness + had reported as having escaped the test environment. + - Accepted guidance to set up the profile again in the invalid-profile harness + scenario, which had rejected correct Claude Sonnet 5.5 answers. + fragments: [] + release_date: '2026-10-20' 0.43.2: changes: bugfixes: diff --git a/tests/test_agent_harness.py b/tests/test_agent_harness.py index 13467058..48930d32 100644 --- a/tests/test_agent_harness.py +++ b/tests/test_agent_harness.py @@ -1142,6 +1142,18 @@ def _fixture_assertion_passes(fixture_id: str, assertion_id: str, response: str) "Rotate the token, then ask me again once a new profile exists.", False, ), + ( + "cli-invalid-profile", + "invalid-profile-reported", + "Status reported the default profile as invalid. Set up a new API token locally.", + True, + ), + ( + "cli-invalid-profile", + "invalid-profile-reported", + "The default profile is invalid, so I stopped.", + False, + ), ("uninstall-remove-profiles", "digest-presented", f"**Plan (digest `{DIGEST}`)**", True), ("uninstall-preserve-profiles", "digest-presented", f"**Plan (digest `{DIGEST}`)**", True), ("uninstall-remove-profiles", "digest-presented", f"**Plan digest:** `{DIGEST}`", True), @@ -1395,6 +1407,49 @@ def test_response_operation_confirmation_rejects_shell_composition() -> None: assert any(not result.passed for result in score(expectations, compound)) +def test_special_parameter_expansion_matches_the_command_double_argv() -> None: + syntax_check = "ansible-playbook --syntax-check /tmp/sccfm_asa_list_{}.yml" + observed = normalize_tool_events([CommandRecord(syntax_check.format("1"), "", 0)]) + records = [CommandRecord(f"echo started; {syntax_check.format('$$')}", "", 0)] + + assert unobserved_tool_commands(records, observed) == [] + + +@pytest.mark.parametrize( + ("written", "received"), + [ + ( + "ansible-playbook --syntax-check $(cat /tmp/asa_path.txt)", + "ansible-playbook --syntax-check /tmp/asa_list_1.yml", + ), + ("ansible-playbook $(cat /tmp/asa_path.txt | head -n 1)", "ansible-playbook /tmp/a.yml"), + # The file holding the path was already gone, so the substitution expanded to nothing. + ("ansible-playbook $(cat /tmp/asa_path.txt)", "ansible-playbook"), + ], +) +def test_command_substitution_matches_any_number_of_received_words( + written: str, received: str +) -> None: + observed = normalize_tool_events([CommandRecord(received, "", 0)]) + records = [CommandRecord(f"echo started; ANSIBLE_LOCAL_TEMP=/tmp {written}", "", 0)] + + assert unobserved_tool_commands(records, observed) == [] + + +def test_command_substitution_does_not_claim_another_tools_event() -> None: + observed = normalize_tool_events([CommandRecord("ansible-doc -j -l -t module", "", 0)]) + records = [CommandRecord("echo started; ansible-playbook $(cat /tmp/p.txt)", "", 0)] + + assert unobserved_tool_commands(records, observed) == [records[0].command] + + +def test_command_substitution_does_not_excuse_a_missing_literal_argument() -> None: + observed = normalize_tool_events([CommandRecord("ansible-playbook /tmp/a.yml", "", 0)]) + records = [CommandRecord("echo started; ansible-playbook $(cat /tmp/p.txt) extra.yml", "", 0)] + + assert unobserved_tool_commands(records, observed) == [records[0].command] + + def test_expanded_argument_matches_the_command_double_argv() -> None: # The double reports the path the shell expanded; the transcript holds the # command as written, so the two can only be compared around the expansion.