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/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/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..6d73e6c 100644 --- a/src/specleft/mcp/payloads.py +++ b/src/specleft/mcp/payloads.py @@ -72,26 +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_cli_api = guarantees.get("cli_api") - cli_api = dict(raw_cli_api) if isinstance(raw_cli_api, 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, - } - guarantees["exit_codes"] = exit_codes - - mcp_payload = dict(payload) - mcp_payload["guarantees"] = guarantees - mcp_payload.pop("docs", None) - return mcp_payload + return build_contract_payload() def build_mcp_guide_payload() -> dict[str, object]: @@ -161,4 +142,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..617851f 100644 --- a/src/specleft/templates/skill_template.py +++ b/src/specleft/templates/skill_template.py @@ -32,7 +32,10 @@ def get_skill_content() -> str: ## 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-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/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_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/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..03e02c5 100644 --- a/tests/mcp/test_server.py +++ b/tests/mcp/test_server.py @@ -61,6 +61,14 @@ async def test_contract_and_guide_resources_are_json(mcp_client: Any) -> None: assert contract_payload["contract_version"] 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 + 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 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: