From afe506f8008e0a7834346c701106041ef1b1ff2f Mon Sep 17 00:00:00 2001 From: Richard-Otterli Date: Mon, 16 Feb 2026 14:30:43 +0000 Subject: [PATCH 1/6] Harden MCP security and CLI input validation (#91) --- docs/cli-reference.md | 1 + src/specleft/commands/enforce.py | 2 + src/specleft/commands/features.py | 148 ++++++++++++++++-- src/specleft/commands/init.py | 16 +- src/specleft/commands/input_validation.py | 60 +++++++ src/specleft/commands/next.py | 15 +- src/specleft/commands/status.py | 15 +- src/specleft/mcp/init_tool.py | 5 + src/specleft/mcp/payloads.py | 51 +++++- src/specleft/templates/skill_template.py | 6 +- src/specleft/utils/skill_integrity.py | 42 +++-- .../test_feature-9-cli-feature-authoring.py | 4 +- tests/commands/test_enforce.py | 11 ++ tests/commands/test_features_add.py | 27 +++- tests/commands/test_init.py | 20 ++- tests/commands/test_next.py | 7 + tests/commands/test_status.py | 7 + tests/mcp/test_security.py | 22 +++ tests/mcp/test_server.py | 6 + 19 files changed, 425 insertions(+), 40 deletions(-) create mode 100644 src/specleft/commands/input_validation.py diff --git a/docs/cli-reference.md b/docs/cli-reference.md index b72ee68..446a91c 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -119,6 +119,7 @@ Options: --example Create example specs --blank Create empty directory structure only --dry-run Show what would be created + --force Regenerate SKILL.md if it was modified --format [table|json] Output format (default: table) ``` diff --git a/src/specleft/commands/enforce.py b/src/specleft/commands/enforce.py index 863a616..fb6b84f 100644 --- a/src/specleft/commands/enforce.py +++ b/src/specleft/commands/enforce.py @@ -12,6 +12,7 @@ import click import yaml +from specleft.commands.input_validation import validate_id_parameter_multiple from specleft.commands.output import json_dumps, resolve_output_format from specleft.specleft_signing.schema import PolicyType, SignedPolicy from specleft.specleft_signing.verify import VerifyFailure, VerifyResult, verify_policy @@ -230,6 +231,7 @@ def _augment_violations_with_fix_commands( "--ignore-feature-id", "ignored", multiple=True, + callback=validate_id_parameter_multiple, help="Exclude feature from evaluation (Enforce only, repeatable).", ) @click.option( diff --git a/src/specleft/commands/features.py b/src/specleft/commands/features.py index 96b286a..04ed88c 100644 --- a/src/specleft/commands/features.py +++ b/src/specleft/commands/features.py @@ -13,6 +13,10 @@ import click +from specleft.commands.input_validation import ( + validate_id_parameter, + validate_text_parameter, +) from specleft.commands.output import json_dumps, resolve_output_format from specleft.commands.test import generate_test_stub from specleft.schema import ( @@ -754,9 +758,15 @@ def features_stats( @click.option( "--id", "feature_id", + callback=validate_id_parameter, help="Feature ID (optional; defaults to a slug from the title).", ) -@click.option("--title", "title", help="Feature title (required).") +@click.option( + "--title", + "title", + callback=validate_text_parameter, + help="Feature title (required).", +) @click.option( "--priority", "priority", @@ -765,7 +775,12 @@ def features_stats( show_default=True, help="Feature priority.", ) -@click.option("--description", "description", help="Feature description.") +@click.option( + "--description", + "description", + callback=validate_text_parameter, + help="Feature description.", +) @click.option( "--dir", "features_dir", @@ -799,14 +814,13 @@ def features_add( if interactive: title_input = click.prompt("Feature title", type=str).strip() - title = title_input default_feature_id = generate_feature_id(title_input) feature_id_input = click.prompt( "Feature ID", default=default_feature_id, show_default=True, ) - feature_id = feature_id_input.strip() + feature_id_value = feature_id_input.strip() priority = click.prompt( "Priority", type=click.Choice([p.value for p in Priority], case_sensitive=False), @@ -814,7 +828,27 @@ def features_add( show_default=True, ) description = click.prompt("Description", default="", show_default=False) - description = description.strip() if description else None + try: + title = validate_text_parameter(None, None, title_input) + feature_id = validate_id_parameter(None, None, feature_id_value) + description = validate_text_parameter( + None, + None, + description.strip() if description else None, + ) + except click.BadParameter as exc: + payload = { + "success": False, + "action": "add", + "error": str(exc), + } + _print_feature_add_result( + result=payload, + format_type=selected_format, + dry_run=dry_run, + pretty=pretty, + ) + sys.exit(1) if not title: click.secho( @@ -837,6 +871,27 @@ def features_add( print_support_footer() sys.exit(1) + try: + feature_id = validate_id_parameter(None, None, feature_id) + title = validate_text_parameter(None, None, title) + description = validate_text_parameter(None, None, description) + except click.BadParameter as exc: + payload = { + "success": False, + "action": "add", + "error": str(exc), + } + _print_feature_add_result( + result=payload, + format_type=selected_format, + dry_run=dry_run, + pretty=pretty, + ) + sys.exit(1) + + assert feature_id is not None + assert title is not None + try: validate_feature_id(feature_id) except ValueError as exc: @@ -895,10 +950,21 @@ def features_add( @click.option( "--feature", "feature_id", + callback=validate_id_parameter, help="Feature ID to append scenario to.", ) -@click.option("--title", "title", help="Scenario title.") -@click.option("--id", "scenario_id", help="Scenario ID (optional).") +@click.option( + "--title", + "title", + callback=validate_text_parameter, + help="Scenario title.", +) +@click.option( + "--id", + "scenario_id", + callback=validate_id_parameter, + help="Scenario ID (optional).", +) @click.option( "--step", "steps", @@ -968,16 +1034,15 @@ def features_add_scenario( _ensure_interactive(interactive) if interactive: - feature_id = click.prompt("Feature ID", type=str).strip() + feature_input = click.prompt("Feature ID", type=str).strip() title_input = click.prompt("Scenario title", type=str).strip() - title = title_input default_scenario_id = generate_scenario_id(title_input) scenario_id_input = click.prompt( "Scenario ID", default=default_scenario_id, show_default=True, ) - scenario_id = scenario_id_input.strip() or None + scenario_id_value = scenario_id_input.strip() or None priority = click.prompt( "Priority", type=click.Choice([p.value for p in Priority], case_sensitive=False), @@ -994,6 +1059,26 @@ def features_add_scenario( break steps_list.append(step) steps = tuple(steps_list) + try: + feature_id = validate_id_parameter(None, None, feature_input) + title = validate_text_parameter(None, None, title_input) + scenario_id = validate_id_parameter(None, None, scenario_id_value) + except click.BadParameter as exc: + payload = { + "success": False, + "action": "add_scenario", + "feature_id": feature_input, + "scenario_id": scenario_id_value, + "error": str(exc), + } + _print_scenario_add_result( + result=payload, + format_type=selected_format, + dry_run=dry_run, + warnings=[], + pretty=pretty, + ) + sys.exit(1) if not feature_id or not title: click.secho( @@ -1006,6 +1091,28 @@ def features_add_scenario( assert title is not None + try: + feature_id = validate_id_parameter(None, None, feature_id) + title = validate_text_parameter(None, None, title) + except click.BadParameter as exc: + payload = { + "success": False, + "action": "add_scenario", + "feature_id": feature_id, + "error": str(exc), + } + _print_scenario_add_result( + result=payload, + format_type=selected_format, + dry_run=dry_run, + warnings=[], + pretty=pretty, + ) + sys.exit(1) + + assert feature_id is not None + assert title is not None + try: validate_feature_id(feature_id) except ValueError as exc: @@ -1028,6 +1135,27 @@ def features_add_scenario( warnings = validate_step_keywords(steps_list) if steps_list else [] scenario_id = scenario_id or generate_scenario_id(title) + try: + scenario_id = validate_id_parameter(None, None, scenario_id) + except click.BadParameter as exc: + payload = { + "success": False, + "action": "add_scenario", + "feature_id": feature_id, + "scenario_id": scenario_id, + "error": str(exc), + } + _print_scenario_add_result( + result=payload, + format_type=selected_format, + dry_run=dry_run, + warnings=warnings, + pretty=pretty, + ) + sys.exit(1) + + assert scenario_id is not None + try: validate_scenario_id(scenario_id) except ValueError as exc: diff --git a/src/specleft/commands/init.py b/src/specleft/commands/init.py index f62eb92..db54ce4 100644 --- a/src/specleft/commands/init.py +++ b/src/specleft/commands/init.py @@ -182,6 +182,11 @@ def _apply_init_plan( @click.option("--example", is_flag=True, help="Create example feature specs.") @click.option("--blank", is_flag=True, help="Create empty directory structure only.") @click.option("--dry-run", is_flag=True, help="Show what would be created.") +@click.option( + "--force", + is_flag=True, + help="Regenerate SKILL.md even if it was modified.", +) @click.option( "--format", "format_type", @@ -191,7 +196,12 @@ def _apply_init_plan( ) @click.option("--pretty", is_flag=True, help="Pretty-print JSON output.") def init( - example: bool, blank: bool, dry_run: bool, format_type: str | None, pretty: bool + example: bool, + blank: bool, + dry_run: bool, + force: bool, + format_type: str | None, + pretty: bool, ) -> None: """Initialize SpecLeft project directories and example specs.""" selected_format = resolve_output_format(format_type) @@ -261,7 +271,7 @@ def init( ) click.echo("") created = _apply_init_plan(directories, files) - skill_sync = sync_skill_files(overwrite_existing=False) + skill_sync = sync_skill_files(overwrite_existing=force) if selected_format == "json": json_payload: dict[str, object] = { @@ -269,6 +279,8 @@ def init( "health": {"ok": True}, "skill_file": str(SKILL_FILE_PATH), "skill_file_hash": skill_sync.skill_file_hash, + "skill_file_regenerated": skill_sync.skill_file_regenerated, + "warnings": skill_sync.warnings, } click.echo(json_dumps(json_payload, pretty=pretty)) return diff --git a/src/specleft/commands/input_validation.py b/src/specleft/commands/input_validation.py new file mode 100644 index 0000000..d55adc0 --- /dev/null +++ b/src/specleft/commands/input_validation.py @@ -0,0 +1,60 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright (c) 2026 SpecLeft Contributors + +"""Input validation callbacks for security-sensitive CLI parameters.""" + +from __future__ import annotations + +import re + +import click + +_ID_PATTERN = re.compile(r"^[a-z0-9]+(?:-[a-z0-9]+)*$") +_SHELL_METACHARACTERS = set("$`|;&><(){}[]!\\'\"") + + +def validate_id_parameter( + _ctx: click.Context | None, + _param: click.Parameter | None, + value: str | None, +) -> str | None: + """Enforce strict kebab-case IDs for CLI parameters.""" + if value is None: + return value + if not _ID_PATTERN.fullmatch(value): + raise click.BadParameter( + f"Must be kebab-case alphanumeric (got: {value!r}). " + "Characters allowed: a-z, 0-9, hyphens." + ) + return value + + +def validate_id_parameter_multiple( + _ctx: click.Context | None, + _param: click.Parameter | None, + value: tuple[str, ...] | None, +) -> tuple[str, ...] | None: + """Enforce strict kebab-case IDs for repeatable CLI parameters.""" + if value is None: + return value + for item in value: + validate_id_parameter(None, None, item) + return value + + +def validate_text_parameter( + _ctx: click.Context | None, + _param: click.Parameter | None, + value: str | None, +) -> str | None: + """Reject shell metacharacters in freeform text CLI parameters.""" + if value is None: + return value + dangerous = sorted(char for char in set(value) if char in _SHELL_METACHARACTERS) + if dangerous: + rendered = ", ".join(repr(char) for char in dangerous) + raise click.BadParameter( + f"Contains disallowed characters: {rendered}. " + "Avoid shell metacharacters in text inputs." + ) + return value diff --git a/src/specleft/commands/next.py b/src/specleft/commands/next.py index 5994e03..18237ea 100644 --- a/src/specleft/commands/next.py +++ b/src/specleft/commands/next.py @@ -13,6 +13,7 @@ import click from specleft.commands.formatters import get_priority_value +from specleft.commands.input_validation import validate_id_parameter from specleft.commands.output import ( compact_mode_enabled, json_dumps, @@ -144,8 +145,18 @@ def _build_next_json( type=click.Choice(["critical", "high", "medium", "low"], case_sensitive=False), help="Filter by priority.", ) -@click.option("--feature", "feature_id", help="Filter by feature ID.") -@click.option("--story", "story_id", help="Filter by story ID.") +@click.option( + "--feature", + "feature_id", + callback=validate_id_parameter, + help="Filter by feature ID.", +) +@click.option( + "--story", + "story_id", + callback=validate_id_parameter, + help="Filter by story ID.", +) @click.option("--pretty", is_flag=True, help="Pretty-print JSON output.") def next_command( features_dir: str | None, diff --git a/src/specleft/commands/status.py b/src/specleft/commands/status.py index 5e120c1..37f3ed9 100644 --- a/src/specleft/commands/status.py +++ b/src/specleft/commands/status.py @@ -14,6 +14,7 @@ import click from specleft.commands.formatters import get_priority_value +from specleft.commands.input_validation import validate_id_parameter from specleft.commands.output import json_dumps, resolve_output_format from specleft.commands.types import ScenarioStatus, ScenarioStatusEntry, StatusSummary from specleft.schema import SpecsConfig @@ -382,8 +383,18 @@ def _is_single_file_feature(feature: object) -> bool: default=None, help="Output format. Defaults to table in a terminal and json otherwise.", ) -@click.option("--feature", "feature_id", help="Filter by feature ID.") -@click.option("--story", "story_id", help="Filter by story ID.") +@click.option( + "--feature", + "feature_id", + callback=validate_id_parameter, + help="Filter by feature ID.", +) +@click.option( + "--story", + "story_id", + callback=validate_id_parameter, + help="Filter by story ID.", +) @click.option( "--unimplemented", is_flag=True, help="Show only unimplemented scenarios." ) diff --git a/src/specleft/mcp/init_tool.py b/src/specleft/mcp/init_tool.py index bb30e93..1b5a766 100644 --- a/src/specleft/mcp/init_tool.py +++ b/src/specleft/mcp/init_tool.py @@ -121,6 +121,7 @@ def run_specleft_init( "plugin_registered": False, }, "created": [], + "skill_file_regenerated": False, "skill_file": str(SKILL_FILE_PATH), "next_steps": "Fix the input arguments and retry.", } @@ -135,6 +136,7 @@ def run_specleft_init( "error": "Environment health checks failed.", "health": health, "created": [], + "skill_file_regenerated": False, "skill_file": str(SKILL_FILE_PATH), "next_steps": "Run `specleft doctor` and resolve reported issues.", "errors": doctor_output.get("errors", []), @@ -151,6 +153,7 @@ def run_specleft_init( "error": str(exc), "health": health, "created": [], + "skill_file_regenerated": False, "skill_file": str(SKILL_FILE_PATH), "next_steps": "Review filesystem safety and retry initialisation.", } @@ -164,6 +167,7 @@ def run_specleft_init( "dry_run": True, "health": health, "created": sorted(set(planned)), + "skill_file_regenerated": False, "skill_file": str(SKILL_FILE_PATH), "next_steps": "Run specleft_init without dry_run to write files.", } @@ -176,6 +180,7 @@ def run_specleft_init( "success": True, "health": health, "created": created_paths, + "skill_file_regenerated": skill_sync.skill_file_regenerated, "skill_file": str(SKILL_FILE_PATH), "next_steps": "Read .specleft/SKILL.md for full CLI reference", "warnings": skill_sync.warnings, diff --git a/src/specleft/mcp/payloads.py b/src/specleft/mcp/payloads.py index 2005d19..756c0a9 100644 --- a/src/specleft/mcp/payloads.py +++ b/src/specleft/mcp/payloads.py @@ -75,8 +75,19 @@ def build_mcp_contract_payload() -> dict[str, Any]: payload = build_contract_payload() raw_guarantees = payload.get("guarantees") guarantees = dict(raw_guarantees) if isinstance(raw_guarantees, dict) else {} + raw_safety = guarantees.get("safety") + safety = dict(raw_safety) if isinstance(raw_safety, dict) else {} + raw_execution = guarantees.get("execution") + execution = dict(raw_execution) if isinstance(raw_execution, dict) else {} + raw_determinism = guarantees.get("determinism") + determinism = dict(raw_determinism) if isinstance(raw_determinism, dict) else {} raw_cli_api = guarantees.get("cli_api") cli_api = dict(raw_cli_api) if isinstance(raw_cli_api, dict) else {} + raw_skill_security = guarantees.get("skill_security") + skill_security = ( + dict(raw_skill_security) if isinstance(raw_skill_security, dict) else {} + ) + raw_exit_codes = cli_api.get("exit_codes") if isinstance(raw_exit_codes, dict): exit_codes = dict(raw_exit_codes) @@ -86,12 +97,39 @@ def build_mcp_contract_payload() -> dict[str, Any]: "error": 1, "cancelled": 2, } - guarantees["exit_codes"] = exit_codes - mcp_payload = dict(payload) - mcp_payload["guarantees"] = guarantees - mcp_payload.pop("docs", None) - return mcp_payload + return { + "contract_version": payload.get("contract_version"), + "specleft_version": payload.get("specleft_version"), + "guarantees": { + "dry_run_never_writes": bool(safety.get("dry_run_never_writes", True)), + "no_writes_without_confirmation": bool( + safety.get("no_implicit_writes", True) + ), + "existing_files_never_overwritten": bool( + safety.get("existing_tests_not_modified_by_default", True) + ), + "skeletons_skipped_by_default": bool( + execution.get("skeletons_skipped_by_default", True) + ), + "skipped_never_fail": bool(execution.get("skipped_never_fail", True)), + "deterministic_for_same_inputs": bool( + determinism.get("deterministic_for_same_inputs", True) + ), + "safe_for_retries": bool(determinism.get("safe_for_retries", True)), + "exit_codes": exit_codes, + "skill_file_integrity_check": bool( + skill_security.get("skill_file_integrity_check", True) + ), + "skill_file_commands_are_simple": bool( + skill_security.get("skill_file_commands_are_simple", True) + ), + "cli_rejects_shell_metacharacters": True, + "init_refuses_symlinks": True, + "no_network_access": True, + "no_telemetry": True, + }, + } def build_mcp_guide_payload() -> dict[str, object]: @@ -161,4 +199,7 @@ def build_mcp_guide_payload() -> dict[str, object]: ], }, "skill_file": "Run specleft_init to generate .specleft/SKILL.md with full CLI reference", + "security_notes": [ + "Avoid sensitive data in feature and scenario names.", + ], } diff --git a/src/specleft/templates/skill_template.py b/src/specleft/templates/skill_template.py index c2ba9e7..73f6b52 100644 --- a/src/specleft/templates/skill_template.py +++ b/src/specleft/templates/skill_template.py @@ -31,8 +31,10 @@ def get_skill_content() -> str: - Status: `specleft status` for progress snapshots ## Safety - - Always `--dry-run` before writing files - - Never use `--force` unless explicitly requested + - All `--id` values must be kebab-case alphanumeric (`a-z`, `0-9`, hyphens) + - All text inputs reject shell metacharacters (`$`, `` ` ``, `|`, `;`, `&`, etc.) + - Never pass unsanitised user input directly as CLI arguments + - All commands are single invocations - no pipes, chaining, or redirects - Exit codes: 0 = success, 1 = error, 2 = cancelled - Commands are deterministic and safe to retry diff --git a/src/specleft/utils/skill_integrity.py b/src/specleft/utils/skill_integrity.py index 99e4f0c..2604387 100644 --- a/src/specleft/utils/skill_integrity.py +++ b/src/specleft/utils/skill_integrity.py @@ -102,6 +102,7 @@ class SkillSyncResult: skipped: list[str] warnings: list[str] skill_file_hash: str + skill_file_regenerated: bool def to_payload(self) -> dict[str, object]: return { @@ -111,6 +112,7 @@ def to_payload(self) -> dict[str, object]: "skipped": self.skipped, "warnings": self.warnings, "skill_file_hash": self.skill_file_hash, + "skill_file_regenerated": self.skill_file_regenerated, } @@ -153,39 +155,60 @@ def sync_skill_files(*, overwrite_existing: bool) -> SkillSyncResult: canonical_hash = _sha256_hex(canonical_content) skill_path = SKILL_FILE_PATH hash_path = SKILL_HASH_PATH + modified_warning = ( + "Existing SKILL.md has been modified from template (checksum mismatch). " + "Use --force to regenerate." + ) + + skill_hash = canonical_hash + allow_hash_sync = True + skill_file_regenerated = False skill_exists = skill_path.exists() if not skill_exists: _write_file(skill_path, canonical_content) created.append(str(skill_path)) - skill_hash = canonical_hash + skill_file_regenerated = True elif overwrite_existing: current_content = skill_path.read_text() if current_content != canonical_content: _write_file(skill_path, canonical_content) updated.append(str(skill_path)) + skill_file_regenerated = True else: skipped.append(str(skill_path)) - skill_hash = canonical_hash else: - skipped.append(str(skill_path)) - warnings.append("Warning: Skipped creation. Specleft SKILL.md exists already.") - skill_hash = _sha256_hex(skill_path.read_text()) + current_content = skill_path.read_text() + current_hash = _sha256_hex(current_content) + expected_hash = _read_hash(hash_path) + skill_hash = current_hash + if expected_hash is None or expected_hash != current_hash: + skipped.append(str(skill_path)) + warnings.append(modified_warning) + allow_hash_sync = False + elif current_hash != canonical_hash: + _write_file(skill_path, canonical_content) + updated.append(str(skill_path)) + skill_hash = canonical_hash + skill_file_regenerated = True + else: + skipped.append(str(skill_path)) hash_exists = hash_path.exists() hash_content = f"{skill_hash}\n" - if not hash_exists: + if not allow_hash_sync: + if hash_exists: + skipped.append(str(hash_path)) + elif not hash_exists: _write_file(hash_path, hash_content) created.append(str(hash_path)) - elif overwrite_existing: + else: current_hash = hash_path.read_text() if current_hash != hash_content: _write_file(hash_path, hash_content) updated.append(str(hash_path)) else: skipped.append(str(hash_path)) - else: - skipped.append(str(hash_path)) return SkillSyncResult( created=created, @@ -193,6 +216,7 @@ def sync_skill_files(*, overwrite_existing: bool) -> SkillSyncResult: skipped=skipped, warnings=warnings, skill_file_hash=skill_hash, + skill_file_regenerated=skill_file_regenerated, ) diff --git a/tests/acceptance/test_feature-9-cli-feature-authoring.py b/tests/acceptance/test_feature-9-cli-feature-authoring.py index 414ea83..b79c4e5 100644 --- a/tests/acceptance/test_feature-9-cli-feature-authoring.py +++ b/tests/acceptance/test_feature-9-cli-feature-authoring.py @@ -104,7 +104,7 @@ def test_reject_invalid_feature_ids( "Bad Id", ], ) - assert result.exit_code == 1, result.output + assert result.exit_code == 2, result.output with specleft.step("And no history entry is recorded"): entries = load_feature_history("Bad Id") @@ -113,7 +113,7 @@ def test_reject_invalid_feature_ids( with specleft.step( "Then the command exits with a validation error and no file is written" ): - assert "Feature ID must match" in result.output + assert "Must be kebab-case alphanumeric" in result.output assert not feature_file.exists() diff --git a/tests/commands/test_enforce.py b/tests/commands/test_enforce.py index 4138bd9..fa99f2a 100644 --- a/tests/commands/test_enforce.py +++ b/tests/commands/test_enforce.py @@ -148,6 +148,17 @@ def test_enforce_core_rejects_ignore_flag(self) -> None: assert result.exit_code == 1 assert "--ignore-feature-id requires Enforce" in result.output + def test_enforce_rejects_invalid_ignore_feature_id(self) -> None: + """--ignore-feature-id validates IDs at Click parsing layer.""" + runner = CliRunner() + with runner.isolated_filesystem(): + result = runner.invoke( + cli, + ["enforce", "--ignore-feature-id", "Bad ID"], + ) + assert result.exit_code == 2 + assert "Must be kebab-case alphanumeric" in result.output + def test_enforce_invalid_signature_exit_2(self) -> None: """Tampered file exits with code 2.""" runner = CliRunner() diff --git a/tests/commands/test_features_add.py b/tests/commands/test_features_add.py index e394921..eef8b96 100644 --- a/tests/commands/test_features_add.py +++ b/tests/commands/test_features_add.py @@ -50,8 +50,27 @@ def test_add_invalid_id(self) -> None: ".specleft/specs", ], ) - assert result.exit_code == 1 - assert "Feature ID must match" in result.output + assert result.exit_code == 2 + assert "Must be kebab-case alphanumeric" in result.output + + def test_add_rejects_shell_metacharacters_in_title(self) -> None: + runner = CliRunner() + with runner.isolated_filesystem(): + result = runner.invoke( + cli, + [ + "features", + "add", + "--id", + "bad-title", + "--title", + "Bad;Title", + "--dir", + ".specleft/specs", + ], + ) + assert result.exit_code == 2 + assert "Contains disallowed characters" in result.output def test_add_dry_run(self) -> None: runner = CliRunner() @@ -304,8 +323,8 @@ def test_add_scenario_invalid_id(self) -> None: ".specleft/specs", ], ) - assert result.exit_code == 1 - assert "Scenario ID must match" in result.output + assert result.exit_code == 2 + assert "Must be kebab-case alphanumeric" in result.output def test_add_scenario_add_test_stub_creates_file(self) -> None: runner = CliRunner() diff --git a/tests/commands/test_init.py b/tests/commands/test_init.py index 688da8c..f210891 100644 --- a/tests/commands/test_init.py +++ b/tests/commands/test_init.py @@ -51,6 +51,8 @@ def test_init_json_supports_non_dry_run(self) -> None: assert result.exit_code == 0 payload = json.loads(result.output) assert payload["success"] is True + assert payload["skill_file_regenerated"] is True + assert payload["warnings"] == [] def test_init_blank_creates_directories(self) -> None: runner = CliRunner() @@ -106,9 +108,23 @@ def test_init_existing_skill_file_warns_and_continues(self) -> None: result = runner.invoke(cli, ["init", "--format", "table"]) assert result.exit_code == 0 assert ( - "Warning: Skipped creation. Specleft SKILL.md exists already." - in result.output + "Existing SKILL.md has been modified from template " + "(checksum mismatch). Use --force to regenerate." in result.output ) assert Path(".specleft/specs/example-feature.md").exists() assert Path(".specleft/SKILL.md").read_text() == "# existing\n" + assert not Path(".specleft/SKILL.md.sha256").exists() + + def test_init_force_regenerates_modified_skill_file(self) -> None: + runner = CliRunner() + with runner.isolated_filesystem(): + Path(".specleft").mkdir(parents=True) + Path(".specleft/SKILL.md").write_text("# existing\n") + + result = runner.invoke(cli, ["init", "--force", "--format", "json"]) + assert result.exit_code == 0 + payload = json.loads(result.output) + assert payload["success"] is True + assert payload["skill_file_regenerated"] is True + assert payload["warnings"] == [] assert Path(".specleft/SKILL.md.sha256").exists() diff --git a/tests/commands/test_next.py b/tests/commands/test_next.py index c719efe..e995706 100644 --- a/tests/commands/test_next.py +++ b/tests/commands/test_next.py @@ -120,3 +120,10 @@ def test_login_success(): result = runner.invoke(cli, ["next"]) assert result.exit_code == 0 assert "All scenarios are implemented" in result.output + + def test_next_rejects_invalid_id_format(self) -> None: + runner = CliRunner() + with runner.isolated_filesystem(): + result = runner.invoke(cli, ["next", "--feature", "Bad ID"]) + assert result.exit_code == 2 + assert "Must be kebab-case alphanumeric" in result.output diff --git a/tests/commands/test_status.py b/tests/commands/test_status.py index c44909b..9b4623f 100644 --- a/tests/commands/test_status.py +++ b/tests/commands/test_status.py @@ -88,6 +88,13 @@ def test_status_filter_requires_valid_ids(self) -> None: assert result.exit_code == 1 assert "Unknown feature ID" in result.output + def test_status_rejects_invalid_id_format(self) -> None: + runner = CliRunner() + with runner.isolated_filesystem(): + result = runner.invoke(cli, ["status", "--feature", "Bad ID"]) + assert result.exit_code == 2 + assert "Must be kebab-case alphanumeric" in result.output + def test_status_treats_missing_test_file_as_skipped(self) -> None: runner = CliRunner() with runner.isolated_filesystem(): diff --git a/tests/mcp/test_security.py b/tests/mcp/test_security.py index 524ed87..45b5f88 100644 --- a/tests/mcp/test_security.py +++ b/tests/mcp/test_security.py @@ -59,3 +59,25 @@ def test_init_generates_verified_read_only_skill_file( integrity = verify_skill_integrity().to_payload() assert integrity["commands_simple"] is True assert integrity["integrity"] in {"pass", "outdated"} + + +def test_init_does_not_regenerate_modified_skill_file_without_force( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.chdir(tmp_path) + skill_file = tmp_path / ".specleft" / "SKILL.md" + checksum_file = tmp_path / ".specleft" / "SKILL.md.sha256" + skill_file.parent.mkdir(parents=True, exist_ok=True) + skill_file.write_text("# tampered\n") + checksum_file.write_text(("a" * 64) + "\n") + + payload = run_specleft_init(blank=True) + + assert payload["success"] is True + assert payload["skill_file_regenerated"] is False + warnings = payload.get("warnings", []) + assert isinstance(warnings, list) + assert warnings + assert "checksum mismatch" in warnings[0].lower() + assert skill_file.read_text() == "# tampered\n" diff --git a/tests/mcp/test_server.py b/tests/mcp/test_server.py index 8096b60..584da4d 100644 --- a/tests/mcp/test_server.py +++ b/tests/mcp/test_server.py @@ -61,6 +61,12 @@ async def test_contract_and_guide_resources_are_json(mcp_client: Any) -> None: assert contract_payload["contract_version"] assert "guarantees" in contract_payload + guarantees = contract_payload["guarantees"] + assert guarantees["cli_rejects_shell_metacharacters"] is True + assert guarantees["init_refuses_symlinks"] is True + assert guarantees["skill_file_integrity_check"] is True + assert guarantees["no_network_access"] is True + assert guarantees["no_telemetry"] is True assert guide_payload["workflow"] assert "skill_file" in guide_payload From 0aba7dfbb59d42e7f0954bf1d5c49757cbb206fd Mon Sep 17 00:00:00 2001 From: Richard-Otterli Date: Mon, 16 Feb 2026 15:25:02 +0000 Subject: [PATCH 2/6] Address PR review feedback on contract payloads (#91) --- src/specleft/commands/contracts/payloads.py | 48 ++++------ src/specleft/commands/contracts/table.py | 49 +++++----- src/specleft/mcp/payloads.py | 59 +----------- src/specleft/templates/skill_template.py | 1 + ..._feature-8-agent-contract-introspection.py | 94 ++++++++----------- tests/commands/test_contract.py | 8 +- tests/commands/test_contract_table.py | 38 ++++---- 7 files changed, 106 insertions(+), 191 deletions(-) diff --git a/src/specleft/commands/contracts/payloads.py b/src/specleft/commands/contracts/payloads.py index e9e0b63..2e89ca1 100644 --- a/src/specleft/commands/contracts/payloads.py +++ b/src/specleft/commands/contracts/payloads.py @@ -5,7 +5,7 @@ from __future__ import annotations -from specleft.commands.constants import CLI_VERSION, CONTRACT_DOC_PATH, CONTRACT_VERSION +from specleft.commands.constants import CLI_VERSION, CONTRACT_VERSION from specleft.commands.contracts.types import ContractCheckResult @@ -27,36 +27,24 @@ def build_contract_payload() -> dict[str, object]: "contract_version": CONTRACT_VERSION, "specleft_version": CLI_VERSION, "guarantees": { - "safety": { - "no_implicit_writes": True, - "dry_run_never_writes": True, - "existing_tests_not_modified_by_default": True, + "dry_run_never_writes": True, + "no_writes_without_confirmation": True, + "existing_files_never_overwritten": True, + "skeletons_skipped_by_default": True, + "skipped_never_fail": True, + "deterministic_for_same_inputs": True, + "safe_for_retries": True, + "exit_codes": { + "success": 0, + "error": 1, + "cancelled": 2, }, - "execution": { - "skeletons_skipped_by_default": True, - "skipped_never_fail": True, - "validation_non_destructive": True, - }, - "determinism": { - "deterministic_for_same_inputs": True, - "safe_for_retries": True, - }, - "cli_api": { - "json_supported_globally": True, - "json_additive_within_minor": True, - "exit_codes": { - "success": 0, - "error": 1, - "cancelled": 2, - }, - }, - "skill_security": { - "skill_file_integrity_check": True, - "skill_file_commands_are_simple": True, - }, - }, - "docs": { - "agent_contract": CONTRACT_DOC_PATH, + "skill_file_integrity_check": True, + "skill_file_commands_are_simple": True, + "cli_rejects_shell_metacharacters": True, + "init_refuses_symlinks": True, + "no_network_access": True, + "no_telemetry": True, }, } diff --git a/src/specleft/commands/contracts/table.py b/src/specleft/commands/contracts/table.py index 8a68616..61c9b53 100644 --- a/src/specleft/commands/contracts/table.py +++ b/src/specleft/commands/contracts/table.py @@ -16,11 +16,7 @@ def print_contract_table(payload: Mapping[str, object]) -> None: guarantees = cast(dict[str, Any], payload.get("guarantees", {})) - safety = cast(dict[str, Any], guarantees.get("safety", {})) - execution = cast(dict[str, Any], guarantees.get("execution", {})) - determinism = cast(dict[str, Any], guarantees.get("determinism", {})) - cli_api = cast(dict[str, Any], guarantees.get("cli_api", {})) - skill_security = cast(dict[str, Any], guarantees.get("skill_security", {})) + exit_codes = cast(dict[str, Any], guarantees.get("exit_codes", {})) click.echo("SpecLeft Agent Contract") click.echo("─" * 40) click.echo(f"Contract version: {payload.get('contract_version')}") @@ -29,73 +25,78 @@ def print_contract_table(payload: Mapping[str, object]) -> None: click.echo("Safety:") click.echo( " - No writes without confirmation or --force" - if safety.get("no_implicit_writes") + if guarantees.get("no_writes_without_confirmation") else " - No implicit writes guarantee missing" ) click.echo( " - --dry-run never writes to disk" - if safety.get("dry_run_never_writes") + if guarantees.get("dry_run_never_writes") else " - --dry-run guarantee missing" ) click.echo( " - Existing tests not modified by default" - if safety.get("existing_tests_not_modified_by_default") + if guarantees.get("existing_files_never_overwritten") else " - Existing test protection missing" ) click.echo("") click.echo("Execution:") click.echo( " - Skeleton tests skipped by default" - if execution.get("skeletons_skipped_by_default") + if guarantees.get("skeletons_skipped_by_default") else " - Skeleton skip guarantee missing" ) click.echo( " - Skipped scenarios never fail tests" - if execution.get("skipped_never_fail") + if guarantees.get("skipped_never_fail") else " - Skip behavior guarantee missing" ) click.echo( - " - Validation is non-destructive" - if execution.get("validation_non_destructive") - else " - Validation guarantee missing" + " - Init refuses symlink targets" + if guarantees.get("init_refuses_symlinks") + else " - Init path safety guarantee missing" ) click.echo("") click.echo("Determinism:") click.echo( " - Commands deterministic for same inputs" - if determinism.get("deterministic_for_same_inputs") + if guarantees.get("deterministic_for_same_inputs") else " - Determinism guarantee missing" ) click.echo( " - Safe to re-run in retry loops" - if determinism.get("safe_for_retries") + if guarantees.get("safe_for_retries") else " - Retry safety guarantee missing" ) click.echo("") click.echo("JSON & CLI:") + success = exit_codes.get("success") + error = exit_codes.get("error") + cancelled = exit_codes.get("cancelled") click.echo( - " - All commands support --format json" - if cli_api.get("json_supported_globally") - else " - JSON support guarantee missing" + f" - Stable exit codes: {success}=success, {error}=error, {cancelled}=cancel" ) click.echo( - " - JSON schema additive within minor versions" - if cli_api.get("json_additive_within_minor") - else " - JSON compatibility guarantee missing" + " - CLI rejects shell metacharacters in arguments" + if guarantees.get("cli_rejects_shell_metacharacters") + else " - CLI input hardening guarantee missing" ) - click.echo(" - Stable exit codes: 0=success, 1=error, 2=cancel") click.echo("") click.echo("Skill Security:") click.echo( " - Skill file integrity is verifiable" - if skill_security.get("skill_file_integrity_check") + if guarantees.get("skill_file_integrity_check") else " - Skill integrity guarantee missing" ) click.echo( " - Skill commands are simple invocations (no shell metacharacters)" - if skill_security.get("skill_file_commands_are_simple") + if guarantees.get("skill_file_commands_are_simple") else " - Skill command simplicity guarantee missing" ) + click.echo( + " - No network access and no telemetry" + if guarantees.get("no_network_access") and guarantees.get("no_telemetry") + else " - Network/telemetry isolation guarantee missing" + ) click.echo("") click.echo(f"For full details, see: {CONTRACT_DOC_PATH}") click.echo("─" * 40) diff --git a/src/specleft/mcp/payloads.py b/src/specleft/mcp/payloads.py index 756c0a9..6d73e6c 100644 --- a/src/specleft/mcp/payloads.py +++ b/src/specleft/mcp/payloads.py @@ -72,64 +72,7 @@ def build_mcp_status_payload( def build_mcp_contract_payload() -> dict[str, Any]: """Build the contract resource payload for MCP clients.""" - payload = build_contract_payload() - raw_guarantees = payload.get("guarantees") - guarantees = dict(raw_guarantees) if isinstance(raw_guarantees, dict) else {} - raw_safety = guarantees.get("safety") - safety = dict(raw_safety) if isinstance(raw_safety, dict) else {} - raw_execution = guarantees.get("execution") - execution = dict(raw_execution) if isinstance(raw_execution, dict) else {} - raw_determinism = guarantees.get("determinism") - determinism = dict(raw_determinism) if isinstance(raw_determinism, dict) else {} - raw_cli_api = guarantees.get("cli_api") - cli_api = dict(raw_cli_api) if isinstance(raw_cli_api, dict) else {} - raw_skill_security = guarantees.get("skill_security") - skill_security = ( - dict(raw_skill_security) if isinstance(raw_skill_security, dict) else {} - ) - - raw_exit_codes = cli_api.get("exit_codes") - if isinstance(raw_exit_codes, dict): - exit_codes = dict(raw_exit_codes) - else: - exit_codes = { - "success": 0, - "error": 1, - "cancelled": 2, - } - - return { - "contract_version": payload.get("contract_version"), - "specleft_version": payload.get("specleft_version"), - "guarantees": { - "dry_run_never_writes": bool(safety.get("dry_run_never_writes", True)), - "no_writes_without_confirmation": bool( - safety.get("no_implicit_writes", True) - ), - "existing_files_never_overwritten": bool( - safety.get("existing_tests_not_modified_by_default", True) - ), - "skeletons_skipped_by_default": bool( - execution.get("skeletons_skipped_by_default", True) - ), - "skipped_never_fail": bool(execution.get("skipped_never_fail", True)), - "deterministic_for_same_inputs": bool( - determinism.get("deterministic_for_same_inputs", True) - ), - "safe_for_retries": bool(determinism.get("safe_for_retries", True)), - "exit_codes": exit_codes, - "skill_file_integrity_check": bool( - skill_security.get("skill_file_integrity_check", True) - ), - "skill_file_commands_are_simple": bool( - skill_security.get("skill_file_commands_are_simple", True) - ), - "cli_rejects_shell_metacharacters": True, - "init_refuses_symlinks": True, - "no_network_access": True, - "no_telemetry": True, - }, - } + return build_contract_payload() def build_mcp_guide_payload() -> dict[str, object]: diff --git a/src/specleft/templates/skill_template.py b/src/specleft/templates/skill_template.py index 73f6b52..617851f 100644 --- a/src/specleft/templates/skill_template.py +++ b/src/specleft/templates/skill_template.py @@ -31,6 +31,7 @@ def get_skill_content() -> str: - Status: `specleft status` for progress snapshots ## Safety + - Always `--dry-run` before writing files - All `--id` values must be kebab-case alphanumeric (`a-z`, `0-9`, hyphens) - All text inputs reject shell metacharacters (`$`, `` ` ``, `|`, `;`, `&`, etc.) - Never pass unsanitised user input directly as CLI arguments diff --git a/tests/acceptance/test_feature-8-agent-contract-introspection.py b/tests/acceptance/test_feature-8-agent-contract-introspection.py index 3883751..344efe7 100644 --- a/tests/acceptance/test_feature-8-agent-contract-introspection.py +++ b/tests/acceptance/test_feature-8-agent-contract-introspection.py @@ -84,47 +84,37 @@ def test_expose_agent_contract_as_structured_json( guarantees = payload["guarantees"] - # Verify safety guarantees - assert ( - "safety" in guarantees - ), f"Expected 'safety' in guarantees. Got: {list(guarantees.keys())}" - safety = guarantees["safety"] - assert ( - "no_implicit_writes" in safety - ), f"Expected 'no_implicit_writes' in safety. Got: {list(safety.keys())}" - assert ( - "dry_run_never_writes" in safety - ), f"Expected 'dry_run_never_writes' in safety. Got: {list(safety.keys())}" - - # Verify execution guarantees - assert ( - "execution" in guarantees - ), f"Expected 'execution' in guarantees. Got: {list(guarantees.keys())}" - execution = guarantees["execution"] - assert ( - "skeletons_skipped_by_default" in execution - ), f"Expected 'skeletons_skipped_by_default'. Got: {list(execution.keys())}" - - # Verify determinism guarantees - assert ( - "determinism" in guarantees - ), f"Expected 'determinism' in guarantees. Got: {list(guarantees.keys())}" - determinism = guarantees["determinism"] - assert ( - "deterministic_for_same_inputs" in determinism - ), f"Expected 'deterministic_for_same_inputs'. Got: {list(determinism.keys())}" + expected_boolean_keys = { + "dry_run_never_writes", + "no_writes_without_confirmation", + "existing_files_never_overwritten", + "skeletons_skipped_by_default", + "skipped_never_fail", + "deterministic_for_same_inputs", + "safe_for_retries", + "skill_file_integrity_check", + "skill_file_commands_are_simple", + "cli_rejects_shell_metacharacters", + "init_refuses_symlinks", + "no_network_access", + "no_telemetry", + } + for key in expected_boolean_keys: + assert ( + key in guarantees + ), f"Expected '{key}' in guarantees. Got: {list(guarantees.keys())}" + assert isinstance(guarantees[key], bool), ( + f"Expected guarantees.{key} to be bool, got " + f"{type(guarantees[key]).__name__}" + ) - # Verify CLI API guarantees assert ( - "cli_api" in guarantees - ), f"Expected 'cli_api' in guarantees. Got: {list(guarantees.keys())}" - cli_api = guarantees["cli_api"] - assert ( - "json_supported_globally" in cli_api - ), f"Expected 'json_supported_globally'. Got: {list(cli_api.keys())}" - assert ( - "exit_codes" in cli_api - ), f"Expected 'exit_codes' in cli_api. Got: {list(cli_api.keys())}" + "exit_codes" in guarantees + ), f"Expected 'exit_codes' in guarantees. Got: {list(guarantees.keys())}" + assert isinstance(guarantees["exit_codes"], dict), ( + f"Expected guarantees.exit_codes to be dict, got " + f"{type(guarantees['exit_codes']).__name__}" + ) with specleft.step("And the JSON schema is stable and machine-friendly"): # Verify version information for schema stability @@ -135,23 +125,17 @@ def test_expose_agent_contract_as_structured_json( "specleft_version" in payload ), f"Expected 'specleft_version'. Got: {list(payload.keys())}" - # Verify all guarantee values are booleans or well-defined structures + # Verify all guarantee values are booleans except exit_codes dict # (machine-friendly means predictable types) - for category, clauses in guarantees.items(): - assert isinstance( - clauses, dict - ), f"Expected {category} to be dict, got {type(clauses).__name__}" - for key, value in clauses.items(): - if key == "exit_codes": - # exit_codes is a nested dict of int values - assert isinstance( - value, dict - ), f"Expected exit_codes to be dict, got {type(value).__name__}" - else: - # Other values should be booleans - assert isinstance( - value, bool - ), f"Expected {category}.{key} to be bool, got {type(value).__name__}" + for key, value in guarantees.items(): + if key == "exit_codes": + assert isinstance( + value, dict + ), f"Expected exit_codes to be dict, got {type(value).__name__}" + else: + assert isinstance( + value, bool + ), f"Expected guarantees.{key} to be bool, got {type(value).__name__}" @specleft( diff --git a/tests/commands/test_contract.py b/tests/commands/test_contract.py index 370f90d..ca01b54 100644 --- a/tests/commands/test_contract.py +++ b/tests/commands/test_contract.py @@ -20,9 +20,11 @@ def test_contract_json_output(self) -> None: assert payload["contract_version"] == "1.1" assert payload["specleft_version"] == CLI_VERSION assert "guarantees" in payload - skill_security = payload["guarantees"]["skill_security"] - assert skill_security["skill_file_integrity_check"] is True - assert skill_security["skill_file_commands_are_simple"] is True + guarantees = payload["guarantees"] + assert guarantees["skill_file_integrity_check"] is True + assert guarantees["skill_file_commands_are_simple"] is True + assert guarantees["cli_rejects_shell_metacharacters"] is True + assert guarantees["init_refuses_symlinks"] is True def test_contract_test_json_output(self) -> None: runner = CliRunner() diff --git a/tests/commands/test_contract_table.py b/tests/commands/test_contract_table.py index 96767a5..a742926 100644 --- a/tests/commands/test_contract_table.py +++ b/tests/commands/test_contract_table.py @@ -39,28 +39,24 @@ def test_print_contract_table_outputs_sections( "contract_version": "1.1", "specleft_version": "0.2.0", "guarantees": { - "safety": { - "no_implicit_writes": True, - "dry_run_never_writes": True, - "existing_tests_not_modified_by_default": True, - }, - "execution": { - "skeletons_skipped_by_default": True, - "skipped_never_fail": True, - "validation_non_destructive": True, - }, - "determinism": { - "deterministic_for_same_inputs": True, - "safe_for_retries": True, - }, - "cli_api": { - "json_supported_globally": True, - "json_additive_within_minor": True, - }, - "skill_security": { - "skill_file_integrity_check": True, - "skill_file_commands_are_simple": True, + "dry_run_never_writes": True, + "no_writes_without_confirmation": True, + "existing_files_never_overwritten": True, + "skeletons_skipped_by_default": True, + "skipped_never_fail": True, + "deterministic_for_same_inputs": True, + "safe_for_retries": True, + "exit_codes": { + "success": 0, + "error": 1, + "cancelled": 2, }, + "skill_file_integrity_check": True, + "skill_file_commands_are_simple": True, + "cli_rejects_shell_metacharacters": True, + "init_refuses_symlinks": True, + "no_network_access": True, + "no_telemetry": True, }, } print_contract_table(payload) From 675406b2f98970985435d271e68a6feb38c0257b Mon Sep 17 00:00:00 2001 From: Richard-Otterli Date: Mon, 16 Feb 2026 15:31:31 +0000 Subject: [PATCH 3/6] Remove guarantees wrapper from contract payload (#91) --- src/specleft/commands/contracts/payloads.py | 36 +++++++++---------- src/specleft/commands/contracts/runner.py | 3 +- src/specleft/commands/contracts/table.py | 27 +++++++------- ..._feature-8-agent-contract-introspection.py | 33 +++++++++-------- tests/commands/test_contract.py | 11 +++--- tests/commands/test_contract_table.py | 36 +++++++++---------- tests/mcp/test_server.py | 15 ++++---- 7 files changed, 77 insertions(+), 84 deletions(-) diff --git a/src/specleft/commands/contracts/payloads.py b/src/specleft/commands/contracts/payloads.py index 2e89ca1..ed4fb60 100644 --- a/src/specleft/commands/contracts/payloads.py +++ b/src/specleft/commands/contracts/payloads.py @@ -26,26 +26,24 @@ def build_contract_payload() -> dict[str, object]: return { "contract_version": CONTRACT_VERSION, "specleft_version": CLI_VERSION, - "guarantees": { - "dry_run_never_writes": True, - "no_writes_without_confirmation": True, - "existing_files_never_overwritten": True, - "skeletons_skipped_by_default": True, - "skipped_never_fail": True, - "deterministic_for_same_inputs": True, - "safe_for_retries": True, - "exit_codes": { - "success": 0, - "error": 1, - "cancelled": 2, - }, - "skill_file_integrity_check": True, - "skill_file_commands_are_simple": True, - "cli_rejects_shell_metacharacters": True, - "init_refuses_symlinks": True, - "no_network_access": True, - "no_telemetry": True, + "dry_run_never_writes": True, + "no_writes_without_confirmation": True, + "existing_files_never_overwritten": True, + "skeletons_skipped_by_default": True, + "skipped_never_fail": True, + "deterministic_for_same_inputs": True, + "safe_for_retries": True, + "exit_codes": { + "success": 0, + "error": 1, + "cancelled": 2, }, + "skill_file_integrity_check": True, + "skill_file_commands_are_simple": True, + "cli_rejects_shell_metacharacters": True, + "init_refuses_symlinks": True, + "no_network_access": True, + "no_telemetry": True, } diff --git a/src/specleft/commands/contracts/runner.py b/src/specleft/commands/contracts/runner.py index 15d0fb0..f7d3212 100644 --- a/src/specleft/commands/contracts/runner.py +++ b/src/specleft/commands/contracts/runner.py @@ -254,7 +254,8 @@ def _normalize_payload(raw_output: str) -> dict[str, object] | None: schema_pass = isinstance(contract_payload, dict) and bool( contract_payload.get("contract_version") and contract_payload.get("specleft_version") - and contract_payload.get("guarantees") + and contract_payload.get("dry_run_never_writes") is not None + and contract_payload.get("exit_codes") ) _record_check( ContractCheckResult( diff --git a/src/specleft/commands/contracts/table.py b/src/specleft/commands/contracts/table.py index 61c9b53..36b3db5 100644 --- a/src/specleft/commands/contracts/table.py +++ b/src/specleft/commands/contracts/table.py @@ -15,8 +15,7 @@ def print_contract_table(payload: Mapping[str, object]) -> None: - guarantees = cast(dict[str, Any], payload.get("guarantees", {})) - exit_codes = cast(dict[str, Any], guarantees.get("exit_codes", {})) + exit_codes = cast(dict[str, Any], payload.get("exit_codes", {})) click.echo("SpecLeft Agent Contract") click.echo("─" * 40) click.echo(f"Contract version: {payload.get('contract_version')}") @@ -25,46 +24,46 @@ def print_contract_table(payload: Mapping[str, object]) -> None: click.echo("Safety:") click.echo( " - No writes without confirmation or --force" - if guarantees.get("no_writes_without_confirmation") + if payload.get("no_writes_without_confirmation") else " - No implicit writes guarantee missing" ) click.echo( " - --dry-run never writes to disk" - if guarantees.get("dry_run_never_writes") + if payload.get("dry_run_never_writes") else " - --dry-run guarantee missing" ) click.echo( " - Existing tests not modified by default" - if guarantees.get("existing_files_never_overwritten") + if payload.get("existing_files_never_overwritten") else " - Existing test protection missing" ) click.echo("") click.echo("Execution:") click.echo( " - Skeleton tests skipped by default" - if guarantees.get("skeletons_skipped_by_default") + if payload.get("skeletons_skipped_by_default") else " - Skeleton skip guarantee missing" ) click.echo( " - Skipped scenarios never fail tests" - if guarantees.get("skipped_never_fail") + if payload.get("skipped_never_fail") else " - Skip behavior guarantee missing" ) click.echo( " - Init refuses symlink targets" - if guarantees.get("init_refuses_symlinks") + if payload.get("init_refuses_symlinks") else " - Init path safety guarantee missing" ) click.echo("") click.echo("Determinism:") click.echo( " - Commands deterministic for same inputs" - if guarantees.get("deterministic_for_same_inputs") + if payload.get("deterministic_for_same_inputs") else " - Determinism guarantee missing" ) click.echo( " - Safe to re-run in retry loops" - if guarantees.get("safe_for_retries") + if payload.get("safe_for_retries") else " - Retry safety guarantee missing" ) click.echo("") @@ -77,24 +76,24 @@ def print_contract_table(payload: Mapping[str, object]) -> None: ) click.echo( " - CLI rejects shell metacharacters in arguments" - if guarantees.get("cli_rejects_shell_metacharacters") + if payload.get("cli_rejects_shell_metacharacters") else " - CLI input hardening guarantee missing" ) click.echo("") click.echo("Skill Security:") click.echo( " - Skill file integrity is verifiable" - if guarantees.get("skill_file_integrity_check") + if payload.get("skill_file_integrity_check") else " - Skill integrity guarantee missing" ) click.echo( " - Skill commands are simple invocations (no shell metacharacters)" - if guarantees.get("skill_file_commands_are_simple") + if payload.get("skill_file_commands_are_simple") else " - Skill command simplicity guarantee missing" ) click.echo( " - No network access and no telemetry" - if guarantees.get("no_network_access") and guarantees.get("no_telemetry") + if payload.get("no_network_access") and payload.get("no_telemetry") else " - Network/telemetry isolation guarantee missing" ) click.echo("") diff --git a/tests/acceptance/test_feature-8-agent-contract-introspection.py b/tests/acceptance/test_feature-8-agent-contract-introspection.py index 344efe7..c2d1c1e 100644 --- a/tests/acceptance/test_feature-8-agent-contract-introspection.py +++ b/tests/acceptance/test_feature-8-agent-contract-introspection.py @@ -77,12 +77,9 @@ def test_expose_agent_contract_as_structured_json( payload, dict ), f"Expected JSON object, got {type(payload).__name__}" - # Verify contract clauses (guarantees) are present assert ( - "guarantees" in payload - ), f"Expected 'guarantees' in contract. Got keys: {list(payload.keys())}" - - guarantees = payload["guarantees"] + "guarantees" not in payload + ), f"Contract payload should not include 'guarantees'. Got keys: {list(payload.keys())}" expected_boolean_keys = { "dry_run_never_writes", @@ -101,19 +98,19 @@ def test_expose_agent_contract_as_structured_json( } for key in expected_boolean_keys: assert ( - key in guarantees - ), f"Expected '{key}' in guarantees. Got: {list(guarantees.keys())}" - assert isinstance(guarantees[key], bool), ( - f"Expected guarantees.{key} to be bool, got " - f"{type(guarantees[key]).__name__}" + key in payload + ), f"Expected '{key}' in payload. Got: {list(payload.keys())}" + assert isinstance(payload[key], bool), ( + f"Expected payload.{key} to be bool, got " + f"{type(payload[key]).__name__}" ) assert ( - "exit_codes" in guarantees - ), f"Expected 'exit_codes' in guarantees. Got: {list(guarantees.keys())}" - assert isinstance(guarantees["exit_codes"], dict), ( - f"Expected guarantees.exit_codes to be dict, got " - f"{type(guarantees['exit_codes']).__name__}" + "exit_codes" in payload + ), f"Expected 'exit_codes' in payload. Got: {list(payload.keys())}" + assert isinstance(payload["exit_codes"], dict), ( + f"Expected payload.exit_codes to be dict, got " + f"{type(payload['exit_codes']).__name__}" ) with specleft.step("And the JSON schema is stable and machine-friendly"): @@ -127,7 +124,9 @@ def test_expose_agent_contract_as_structured_json( # Verify all guarantee values are booleans except exit_codes dict # (machine-friendly means predictable types) - for key, value in guarantees.items(): + for key, value in payload.items(): + if key in {"contract_version", "specleft_version"}: + continue if key == "exit_codes": assert isinstance( value, dict @@ -135,7 +134,7 @@ def test_expose_agent_contract_as_structured_json( else: assert isinstance( value, bool - ), f"Expected guarantees.{key} to be bool, got {type(value).__name__}" + ), f"Expected payload.{key} to be bool, got {type(value).__name__}" @specleft( diff --git a/tests/commands/test_contract.py b/tests/commands/test_contract.py index ca01b54..823d033 100644 --- a/tests/commands/test_contract.py +++ b/tests/commands/test_contract.py @@ -19,12 +19,11 @@ def test_contract_json_output(self) -> None: payload = json.loads(result.output) assert payload["contract_version"] == "1.1" assert payload["specleft_version"] == CLI_VERSION - assert "guarantees" in payload - guarantees = payload["guarantees"] - assert guarantees["skill_file_integrity_check"] is True - assert guarantees["skill_file_commands_are_simple"] is True - assert guarantees["cli_rejects_shell_metacharacters"] is True - assert guarantees["init_refuses_symlinks"] is True + assert "guarantees" not in payload + assert payload["skill_file_integrity_check"] is True + assert payload["skill_file_commands_are_simple"] is True + assert payload["cli_rejects_shell_metacharacters"] is True + assert payload["init_refuses_symlinks"] is True def test_contract_test_json_output(self) -> None: runner = CliRunner() diff --git a/tests/commands/test_contract_table.py b/tests/commands/test_contract_table.py index a742926..0259347 100644 --- a/tests/commands/test_contract_table.py +++ b/tests/commands/test_contract_table.py @@ -38,26 +38,24 @@ def test_print_contract_table_outputs_sections( payload = { "contract_version": "1.1", "specleft_version": "0.2.0", - "guarantees": { - "dry_run_never_writes": True, - "no_writes_without_confirmation": True, - "existing_files_never_overwritten": True, - "skeletons_skipped_by_default": True, - "skipped_never_fail": True, - "deterministic_for_same_inputs": True, - "safe_for_retries": True, - "exit_codes": { - "success": 0, - "error": 1, - "cancelled": 2, - }, - "skill_file_integrity_check": True, - "skill_file_commands_are_simple": True, - "cli_rejects_shell_metacharacters": True, - "init_refuses_symlinks": True, - "no_network_access": True, - "no_telemetry": True, + "dry_run_never_writes": True, + "no_writes_without_confirmation": True, + "existing_files_never_overwritten": True, + "skeletons_skipped_by_default": True, + "skipped_never_fail": True, + "deterministic_for_same_inputs": True, + "safe_for_retries": True, + "exit_codes": { + "success": 0, + "error": 1, + "cancelled": 2, }, + "skill_file_integrity_check": True, + "skill_file_commands_are_simple": True, + "cli_rejects_shell_metacharacters": True, + "init_refuses_symlinks": True, + "no_network_access": True, + "no_telemetry": True, } print_contract_table(payload) output = capsys.readouterr().out diff --git a/tests/mcp/test_server.py b/tests/mcp/test_server.py index 584da4d..34a9287 100644 --- a/tests/mcp/test_server.py +++ b/tests/mcp/test_server.py @@ -60,13 +60,12 @@ async def test_contract_and_guide_resources_are_json(mcp_client: Any) -> None: guide_payload = _resource_json(guide_result) assert contract_payload["contract_version"] - assert "guarantees" in contract_payload - guarantees = contract_payload["guarantees"] - assert guarantees["cli_rejects_shell_metacharacters"] is True - assert guarantees["init_refuses_symlinks"] is True - assert guarantees["skill_file_integrity_check"] is True - assert guarantees["no_network_access"] is True - assert guarantees["no_telemetry"] is True + assert "guarantees" not in contract_payload + assert contract_payload["cli_rejects_shell_metacharacters"] is True + assert contract_payload["init_refuses_symlinks"] is True + assert contract_payload["skill_file_integrity_check"] is True + assert contract_payload["no_network_access"] is True + assert contract_payload["no_telemetry"] is True assert guide_payload["workflow"] assert "skill_file" in guide_payload @@ -97,7 +96,7 @@ async def test_agent_discovery_flow_uses_resources_and_init_tool( status_payload = _resource_json(status_result) with specleft.step("Then status reports initialised=false before setup"): - assert "guarantees" in contract_payload + assert contract_payload["dry_run_never_writes"] is True assert "workflow" in guide_payload assert status_payload["initialised"] is False From 8a59cc97cf79f402727d1175cab3cd1b9ec3b72d Mon Sep 17 00:00:00 2001 From: Richard-Otterli Date: Mon, 16 Feb 2026 15:38:15 +0000 Subject: [PATCH 4/6] Restore nested contract payload structure (#91) --- src/specleft/commands/contracts/payloads.py | 50 ++++++--- src/specleft/commands/contracts/runner.py | 3 +- src/specleft/commands/contracts/table.py | 50 ++++----- ..._feature-8-agent-contract-introspection.py | 105 ++++++++++-------- tests/commands/test_contract.py | 9 +- tests/commands/test_contract_table.py | 40 ++++--- tests/mcp/test_server.py | 12 +- 7 files changed, 151 insertions(+), 118 deletions(-) diff --git a/src/specleft/commands/contracts/payloads.py b/src/specleft/commands/contracts/payloads.py index ed4fb60..e9e0b63 100644 --- a/src/specleft/commands/contracts/payloads.py +++ b/src/specleft/commands/contracts/payloads.py @@ -5,7 +5,7 @@ from __future__ import annotations -from specleft.commands.constants import CLI_VERSION, CONTRACT_VERSION +from specleft.commands.constants import CLI_VERSION, CONTRACT_DOC_PATH, CONTRACT_VERSION from specleft.commands.contracts.types import ContractCheckResult @@ -26,24 +26,38 @@ def build_contract_payload() -> dict[str, object]: return { "contract_version": CONTRACT_VERSION, "specleft_version": CLI_VERSION, - "dry_run_never_writes": True, - "no_writes_without_confirmation": True, - "existing_files_never_overwritten": True, - "skeletons_skipped_by_default": True, - "skipped_never_fail": True, - "deterministic_for_same_inputs": True, - "safe_for_retries": True, - "exit_codes": { - "success": 0, - "error": 1, - "cancelled": 2, + "guarantees": { + "safety": { + "no_implicit_writes": True, + "dry_run_never_writes": True, + "existing_tests_not_modified_by_default": True, + }, + "execution": { + "skeletons_skipped_by_default": True, + "skipped_never_fail": True, + "validation_non_destructive": True, + }, + "determinism": { + "deterministic_for_same_inputs": True, + "safe_for_retries": True, + }, + "cli_api": { + "json_supported_globally": True, + "json_additive_within_minor": True, + "exit_codes": { + "success": 0, + "error": 1, + "cancelled": 2, + }, + }, + "skill_security": { + "skill_file_integrity_check": True, + "skill_file_commands_are_simple": True, + }, + }, + "docs": { + "agent_contract": CONTRACT_DOC_PATH, }, - "skill_file_integrity_check": True, - "skill_file_commands_are_simple": True, - "cli_rejects_shell_metacharacters": True, - "init_refuses_symlinks": True, - "no_network_access": True, - "no_telemetry": True, } diff --git a/src/specleft/commands/contracts/runner.py b/src/specleft/commands/contracts/runner.py index f7d3212..15d0fb0 100644 --- a/src/specleft/commands/contracts/runner.py +++ b/src/specleft/commands/contracts/runner.py @@ -254,8 +254,7 @@ def _normalize_payload(raw_output: str) -> dict[str, object] | None: schema_pass = isinstance(contract_payload, dict) and bool( contract_payload.get("contract_version") and contract_payload.get("specleft_version") - and contract_payload.get("dry_run_never_writes") is not None - and contract_payload.get("exit_codes") + and contract_payload.get("guarantees") ) _record_check( ContractCheckResult( diff --git a/src/specleft/commands/contracts/table.py b/src/specleft/commands/contracts/table.py index 36b3db5..8a68616 100644 --- a/src/specleft/commands/contracts/table.py +++ b/src/specleft/commands/contracts/table.py @@ -15,7 +15,12 @@ def print_contract_table(payload: Mapping[str, object]) -> None: - exit_codes = cast(dict[str, Any], payload.get("exit_codes", {})) + guarantees = cast(dict[str, Any], payload.get("guarantees", {})) + safety = cast(dict[str, Any], guarantees.get("safety", {})) + execution = cast(dict[str, Any], guarantees.get("execution", {})) + determinism = cast(dict[str, Any], guarantees.get("determinism", {})) + cli_api = cast(dict[str, Any], guarantees.get("cli_api", {})) + skill_security = cast(dict[str, Any], guarantees.get("skill_security", {})) click.echo("SpecLeft Agent Contract") click.echo("─" * 40) click.echo(f"Contract version: {payload.get('contract_version')}") @@ -24,78 +29,73 @@ def print_contract_table(payload: Mapping[str, object]) -> None: click.echo("Safety:") click.echo( " - No writes without confirmation or --force" - if payload.get("no_writes_without_confirmation") + if safety.get("no_implicit_writes") else " - No implicit writes guarantee missing" ) click.echo( " - --dry-run never writes to disk" - if payload.get("dry_run_never_writes") + if safety.get("dry_run_never_writes") else " - --dry-run guarantee missing" ) click.echo( " - Existing tests not modified by default" - if payload.get("existing_files_never_overwritten") + if safety.get("existing_tests_not_modified_by_default") else " - Existing test protection missing" ) click.echo("") click.echo("Execution:") click.echo( " - Skeleton tests skipped by default" - if payload.get("skeletons_skipped_by_default") + if execution.get("skeletons_skipped_by_default") else " - Skeleton skip guarantee missing" ) click.echo( " - Skipped scenarios never fail tests" - if payload.get("skipped_never_fail") + if execution.get("skipped_never_fail") else " - Skip behavior guarantee missing" ) click.echo( - " - Init refuses symlink targets" - if payload.get("init_refuses_symlinks") - else " - Init path safety guarantee missing" + " - Validation is non-destructive" + if execution.get("validation_non_destructive") + else " - Validation guarantee missing" ) click.echo("") click.echo("Determinism:") click.echo( " - Commands deterministic for same inputs" - if payload.get("deterministic_for_same_inputs") + if determinism.get("deterministic_for_same_inputs") else " - Determinism guarantee missing" ) click.echo( " - Safe to re-run in retry loops" - if payload.get("safe_for_retries") + if determinism.get("safe_for_retries") else " - Retry safety guarantee missing" ) click.echo("") click.echo("JSON & CLI:") - success = exit_codes.get("success") - error = exit_codes.get("error") - cancelled = exit_codes.get("cancelled") click.echo( - f" - Stable exit codes: {success}=success, {error}=error, {cancelled}=cancel" + " - All commands support --format json" + if cli_api.get("json_supported_globally") + else " - JSON support guarantee missing" ) click.echo( - " - CLI rejects shell metacharacters in arguments" - if payload.get("cli_rejects_shell_metacharacters") - else " - CLI input hardening guarantee missing" + " - JSON schema additive within minor versions" + if cli_api.get("json_additive_within_minor") + else " - JSON compatibility guarantee missing" ) + click.echo(" - Stable exit codes: 0=success, 1=error, 2=cancel") click.echo("") click.echo("Skill Security:") click.echo( " - Skill file integrity is verifiable" - if payload.get("skill_file_integrity_check") + if skill_security.get("skill_file_integrity_check") else " - Skill integrity guarantee missing" ) click.echo( " - Skill commands are simple invocations (no shell metacharacters)" - if payload.get("skill_file_commands_are_simple") + if skill_security.get("skill_file_commands_are_simple") else " - Skill command simplicity guarantee missing" ) - click.echo( - " - No network access and no telemetry" - if payload.get("no_network_access") and payload.get("no_telemetry") - else " - Network/telemetry isolation guarantee missing" - ) click.echo("") click.echo(f"For full details, see: {CONTRACT_DOC_PATH}") click.echo("─" * 40) diff --git a/tests/acceptance/test_feature-8-agent-contract-introspection.py b/tests/acceptance/test_feature-8-agent-contract-introspection.py index c2d1c1e..3883751 100644 --- a/tests/acceptance/test_feature-8-agent-contract-introspection.py +++ b/tests/acceptance/test_feature-8-agent-contract-introspection.py @@ -77,41 +77,54 @@ def test_expose_agent_contract_as_structured_json( payload, dict ), f"Expected JSON object, got {type(payload).__name__}" + # Verify contract clauses (guarantees) are present assert ( - "guarantees" not in payload - ), f"Contract payload should not include 'guarantees'. Got keys: {list(payload.keys())}" - - expected_boolean_keys = { - "dry_run_never_writes", - "no_writes_without_confirmation", - "existing_files_never_overwritten", - "skeletons_skipped_by_default", - "skipped_never_fail", - "deterministic_for_same_inputs", - "safe_for_retries", - "skill_file_integrity_check", - "skill_file_commands_are_simple", - "cli_rejects_shell_metacharacters", - "init_refuses_symlinks", - "no_network_access", - "no_telemetry", - } - for key in expected_boolean_keys: - assert ( - key in payload - ), f"Expected '{key}' in payload. Got: {list(payload.keys())}" - assert isinstance(payload[key], bool), ( - f"Expected payload.{key} to be bool, got " - f"{type(payload[key]).__name__}" - ) + "guarantees" in payload + ), f"Expected 'guarantees' in contract. Got keys: {list(payload.keys())}" + guarantees = payload["guarantees"] + + # Verify safety guarantees assert ( - "exit_codes" in payload - ), f"Expected 'exit_codes' in payload. Got: {list(payload.keys())}" - assert isinstance(payload["exit_codes"], dict), ( - f"Expected payload.exit_codes to be dict, got " - f"{type(payload['exit_codes']).__name__}" - ) + "safety" in guarantees + ), f"Expected 'safety' in guarantees. Got: {list(guarantees.keys())}" + safety = guarantees["safety"] + assert ( + "no_implicit_writes" in safety + ), f"Expected 'no_implicit_writes' in safety. Got: {list(safety.keys())}" + assert ( + "dry_run_never_writes" in safety + ), f"Expected 'dry_run_never_writes' in safety. Got: {list(safety.keys())}" + + # Verify execution guarantees + assert ( + "execution" in guarantees + ), f"Expected 'execution' in guarantees. Got: {list(guarantees.keys())}" + execution = guarantees["execution"] + assert ( + "skeletons_skipped_by_default" in execution + ), f"Expected 'skeletons_skipped_by_default'. Got: {list(execution.keys())}" + + # Verify determinism guarantees + assert ( + "determinism" in guarantees + ), f"Expected 'determinism' in guarantees. Got: {list(guarantees.keys())}" + determinism = guarantees["determinism"] + assert ( + "deterministic_for_same_inputs" in determinism + ), f"Expected 'deterministic_for_same_inputs'. Got: {list(determinism.keys())}" + + # Verify CLI API guarantees + assert ( + "cli_api" in guarantees + ), f"Expected 'cli_api' in guarantees. Got: {list(guarantees.keys())}" + cli_api = guarantees["cli_api"] + assert ( + "json_supported_globally" in cli_api + ), f"Expected 'json_supported_globally'. Got: {list(cli_api.keys())}" + assert ( + "exit_codes" in cli_api + ), f"Expected 'exit_codes' in cli_api. Got: {list(cli_api.keys())}" with specleft.step("And the JSON schema is stable and machine-friendly"): # Verify version information for schema stability @@ -122,19 +135,23 @@ def test_expose_agent_contract_as_structured_json( "specleft_version" in payload ), f"Expected 'specleft_version'. Got: {list(payload.keys())}" - # Verify all guarantee values are booleans except exit_codes dict + # Verify all guarantee values are booleans or well-defined structures # (machine-friendly means predictable types) - for key, value in payload.items(): - if key in {"contract_version", "specleft_version"}: - continue - if key == "exit_codes": - assert isinstance( - value, dict - ), f"Expected exit_codes to be dict, got {type(value).__name__}" - else: - assert isinstance( - value, bool - ), f"Expected payload.{key} to be bool, got {type(value).__name__}" + for category, clauses in guarantees.items(): + assert isinstance( + clauses, dict + ), f"Expected {category} to be dict, got {type(clauses).__name__}" + for key, value in clauses.items(): + if key == "exit_codes": + # exit_codes is a nested dict of int values + assert isinstance( + value, dict + ), f"Expected exit_codes to be dict, got {type(value).__name__}" + else: + # Other values should be booleans + assert isinstance( + value, bool + ), f"Expected {category}.{key} to be bool, got {type(value).__name__}" @specleft( diff --git a/tests/commands/test_contract.py b/tests/commands/test_contract.py index 823d033..370f90d 100644 --- a/tests/commands/test_contract.py +++ b/tests/commands/test_contract.py @@ -19,11 +19,10 @@ def test_contract_json_output(self) -> None: payload = json.loads(result.output) assert payload["contract_version"] == "1.1" assert payload["specleft_version"] == CLI_VERSION - assert "guarantees" not in payload - assert payload["skill_file_integrity_check"] is True - assert payload["skill_file_commands_are_simple"] is True - assert payload["cli_rejects_shell_metacharacters"] is True - assert payload["init_refuses_symlinks"] is True + assert "guarantees" in payload + skill_security = payload["guarantees"]["skill_security"] + assert skill_security["skill_file_integrity_check"] is True + assert skill_security["skill_file_commands_are_simple"] is True def test_contract_test_json_output(self) -> None: runner = CliRunner() diff --git a/tests/commands/test_contract_table.py b/tests/commands/test_contract_table.py index 0259347..96767a5 100644 --- a/tests/commands/test_contract_table.py +++ b/tests/commands/test_contract_table.py @@ -38,24 +38,30 @@ def test_print_contract_table_outputs_sections( payload = { "contract_version": "1.1", "specleft_version": "0.2.0", - "dry_run_never_writes": True, - "no_writes_without_confirmation": True, - "existing_files_never_overwritten": True, - "skeletons_skipped_by_default": True, - "skipped_never_fail": True, - "deterministic_for_same_inputs": True, - "safe_for_retries": True, - "exit_codes": { - "success": 0, - "error": 1, - "cancelled": 2, + "guarantees": { + "safety": { + "no_implicit_writes": True, + "dry_run_never_writes": True, + "existing_tests_not_modified_by_default": True, + }, + "execution": { + "skeletons_skipped_by_default": True, + "skipped_never_fail": True, + "validation_non_destructive": True, + }, + "determinism": { + "deterministic_for_same_inputs": True, + "safe_for_retries": True, + }, + "cli_api": { + "json_supported_globally": True, + "json_additive_within_minor": True, + }, + "skill_security": { + "skill_file_integrity_check": True, + "skill_file_commands_are_simple": True, + }, }, - "skill_file_integrity_check": True, - "skill_file_commands_are_simple": True, - "cli_rejects_shell_metacharacters": True, - "init_refuses_symlinks": True, - "no_network_access": True, - "no_telemetry": True, } print_contract_table(payload) output = capsys.readouterr().out diff --git a/tests/mcp/test_server.py b/tests/mcp/test_server.py index 34a9287..30a12a2 100644 --- a/tests/mcp/test_server.py +++ b/tests/mcp/test_server.py @@ -60,12 +60,10 @@ async def test_contract_and_guide_resources_are_json(mcp_client: Any) -> None: guide_payload = _resource_json(guide_result) assert contract_payload["contract_version"] - assert "guarantees" not in contract_payload - assert contract_payload["cli_rejects_shell_metacharacters"] is True - assert contract_payload["init_refuses_symlinks"] is True - assert contract_payload["skill_file_integrity_check"] is True - assert contract_payload["no_network_access"] is True - assert contract_payload["no_telemetry"] is True + assert "guarantees" in contract_payload + skill_security = contract_payload["guarantees"]["skill_security"] + assert skill_security["skill_file_integrity_check"] is True + assert skill_security["skill_file_commands_are_simple"] is True assert guide_payload["workflow"] assert "skill_file" in guide_payload @@ -96,7 +94,7 @@ async def test_agent_discovery_flow_uses_resources_and_init_tool( status_payload = _resource_json(status_result) with specleft.step("Then status reports initialised=false before setup"): - assert contract_payload["dry_run_never_writes"] is True + assert "guarantees" in contract_payload assert "workflow" in guide_payload assert status_payload["initialised"] is False From 9c5919635e0b64d69f8a158089dfa4b53f700119 Mon Sep 17 00:00:00 2001 From: Richard-Otterli Date: Mon, 16 Feb 2026 15:44:29 +0000 Subject: [PATCH 5/6] Add security guarantees section to contract payload (#91) --- src/specleft/commands/contracts/payloads.py | 6 +++++ src/specleft/commands/contracts/table.py | 23 +++++++++++++++++++ ..._feature-8-agent-contract-introspection.py | 18 +++++++++++++++ tests/commands/test_contract.py | 5 ++++ tests/commands/test_contract_table.py | 7 ++++++ tests/mcp/test_server.py | 5 ++++ 6 files changed, 64 insertions(+) diff --git a/src/specleft/commands/contracts/payloads.py b/src/specleft/commands/contracts/payloads.py index e9e0b63..792d24c 100644 --- a/src/specleft/commands/contracts/payloads.py +++ b/src/specleft/commands/contracts/payloads.py @@ -54,6 +54,12 @@ def build_contract_payload() -> dict[str, object]: "skill_file_integrity_check": True, "skill_file_commands_are_simple": True, }, + "security": { + "cli_rejects_shell_metacharacters": True, + "init_refuses_symlinks": True, + "no_network_access": True, + "no_telemetry": True, + }, }, "docs": { "agent_contract": CONTRACT_DOC_PATH, diff --git a/src/specleft/commands/contracts/table.py b/src/specleft/commands/contracts/table.py index 8a68616..0d51485 100644 --- a/src/specleft/commands/contracts/table.py +++ b/src/specleft/commands/contracts/table.py @@ -21,6 +21,7 @@ def print_contract_table(payload: Mapping[str, object]) -> None: determinism = cast(dict[str, Any], guarantees.get("determinism", {})) cli_api = cast(dict[str, Any], guarantees.get("cli_api", {})) skill_security = cast(dict[str, Any], guarantees.get("skill_security", {})) + security = cast(dict[str, Any], guarantees.get("security", {})) click.echo("SpecLeft Agent Contract") click.echo("─" * 40) click.echo(f"Contract version: {payload.get('contract_version')}") @@ -97,6 +98,28 @@ def print_contract_table(payload: Mapping[str, object]) -> None: else " - Skill command simplicity guarantee missing" ) click.echo("") + click.echo("Security:") + click.echo( + " - CLI rejects shell metacharacters" + if security.get("cli_rejects_shell_metacharacters") + else " - CLI shell metacharacter rejection missing" + ) + click.echo( + " - Init refuses symlink paths" + if security.get("init_refuses_symlinks") + else " - Init symlink safety guarantee missing" + ) + click.echo( + " - No network access" + if security.get("no_network_access") + else " - Network isolation guarantee missing" + ) + click.echo( + " - No telemetry" + if security.get("no_telemetry") + else " - Telemetry isolation guarantee missing" + ) + click.echo("") click.echo(f"For full details, see: {CONTRACT_DOC_PATH}") click.echo("─" * 40) diff --git a/tests/acceptance/test_feature-8-agent-contract-introspection.py b/tests/acceptance/test_feature-8-agent-contract-introspection.py index 3883751..0d41615 100644 --- a/tests/acceptance/test_feature-8-agent-contract-introspection.py +++ b/tests/acceptance/test_feature-8-agent-contract-introspection.py @@ -126,6 +126,24 @@ def test_expose_agent_contract_as_structured_json( "exit_codes" in cli_api ), f"Expected 'exit_codes' in cli_api. Got: {list(cli_api.keys())}" + # Verify security guarantees + assert ( + "security" in guarantees + ), f"Expected 'security' in guarantees. Got: {list(guarantees.keys())}" + security = guarantees["security"] + assert ( + "cli_rejects_shell_metacharacters" in security + ), f"Expected 'cli_rejects_shell_metacharacters'. Got: {list(security.keys())}" + assert ( + "init_refuses_symlinks" in security + ), f"Expected 'init_refuses_symlinks'. Got: {list(security.keys())}" + assert ( + "no_network_access" in security + ), f"Expected 'no_network_access'. Got: {list(security.keys())}" + assert ( + "no_telemetry" in security + ), f"Expected 'no_telemetry'. Got: {list(security.keys())}" + with specleft.step("And the JSON schema is stable and machine-friendly"): # Verify version information for schema stability assert ( diff --git a/tests/commands/test_contract.py b/tests/commands/test_contract.py index 370f90d..0ca4e30 100644 --- a/tests/commands/test_contract.py +++ b/tests/commands/test_contract.py @@ -23,6 +23,11 @@ def test_contract_json_output(self) -> None: skill_security = payload["guarantees"]["skill_security"] assert skill_security["skill_file_integrity_check"] is True assert skill_security["skill_file_commands_are_simple"] is True + security = payload["guarantees"]["security"] + assert security["cli_rejects_shell_metacharacters"] is True + assert security["init_refuses_symlinks"] is True + assert security["no_network_access"] is True + assert security["no_telemetry"] is True def test_contract_test_json_output(self) -> None: runner = CliRunner() diff --git a/tests/commands/test_contract_table.py b/tests/commands/test_contract_table.py index 96767a5..5d1e3b5 100644 --- a/tests/commands/test_contract_table.py +++ b/tests/commands/test_contract_table.py @@ -61,6 +61,12 @@ def test_print_contract_table_outputs_sections( "skill_file_integrity_check": True, "skill_file_commands_are_simple": True, }, + "security": { + "cli_rejects_shell_metacharacters": True, + "init_refuses_symlinks": True, + "no_network_access": True, + "no_telemetry": True, + }, }, } print_contract_table(payload) @@ -71,6 +77,7 @@ def test_print_contract_table_outputs_sections( assert "Determinism:" in output assert "JSON & CLI:" in output assert "Skill Security:" in output + assert "Security:" in output def test_print_contract_test_summary(capsys: pytest.CaptureFixture[str]) -> None: diff --git a/tests/mcp/test_server.py b/tests/mcp/test_server.py index 30a12a2..03e02c5 100644 --- a/tests/mcp/test_server.py +++ b/tests/mcp/test_server.py @@ -64,6 +64,11 @@ async def test_contract_and_guide_resources_are_json(mcp_client: Any) -> None: skill_security = contract_payload["guarantees"]["skill_security"] assert skill_security["skill_file_integrity_check"] is True assert skill_security["skill_file_commands_are_simple"] is True + security = contract_payload["guarantees"]["security"] + assert security["cli_rejects_shell_metacharacters"] is True + assert security["init_refuses_symlinks"] is True + assert security["no_network_access"] is True + assert security["no_telemetry"] is True assert guide_payload["workflow"] assert "skill_file" in guide_payload From abf8ec333a66560ffdbfa27b286197ae0f996df6 Mon Sep 17 00:00:00 2001 From: Richard-Otterli Date: Mon, 16 Feb 2026 15:50:03 +0000 Subject: [PATCH 6/6] Contract token size --- tests/mcp/test_token_budget.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/mcp/test_token_budget.py b/tests/mcp/test_token_budget.py index b24ce56..05fb9f0 100644 --- a/tests/mcp/test_token_budget.py +++ b/tests/mcp/test_token_budget.py @@ -27,7 +27,7 @@ def _compact_json(data: Any) -> str: def test_contract_payload_within_budget() -> None: tokens = _count_tokens(_compact_json(build_mcp_contract_payload())) - assert tokens <= 170 + assert tokens <= 300 def test_guide_payload_within_budget() -> None: