diff --git a/core/lambda/contract_ingestor.py b/core/lambda/contract_ingestor.py index 0b975e8..8880d5d 100644 --- a/core/lambda/contract_ingestor.py +++ b/core/lambda/contract_ingestor.py @@ -30,6 +30,11 @@ PLATFORM_REPO = os.environ.get("PLATFORM_REPO", "nova/acdl") # to a Gitea API root (e.g. https://git.cloudinit.dev/api/v1) for Gitea. GITHUB_API_BASE = os.environ.get("GITHUB_API_BASE", "https://api.github.com") +# P11 (REQ-175): consistent cap for error/stackTrace fields (was 10k vs 2k). +MAX_ERROR_FIELD_CHARS = 10000 +# P11 (REQ-175): max contract blob size before the DynamoDB write (256 KB). +MAX_CONTRACT_BYTES = 256 * 1024 + _dynamodb = None _secrets_client = None @@ -49,6 +54,29 @@ def _discover_environments(): return {"dev", "qa", "prod", "dr"} +def _validate_contract_schema(contract): + """P11 (REQ-175): validate the contract blob against + schemas/contract.schema.json before the DynamoDB write. Raises + ValueError on invalid. Falls back to a no-op if the schema or + jsonschema is unavailable (e.g. packaged Lambda without the schema). + """ + try: + import json as _json + import jsonschema + schema_path = os.path.join(os.path.dirname(os.path.dirname( + os.path.dirname(os.path.abspath(__file__)))), + "schemas", "contract.schema.json") + with open(schema_path) as f: + schema = _json.load(f) + jsonschema.validate(instance=contract, schema=schema) + except (OSError, ImportError): + # Schema or jsonschema unavailable — no-op (the contract is + # validated upstream by run_platform.sh in the normal path). + pass + except jsonschema.ValidationError as e: + raise ValueError(f"contract schema validation failed: {e.message}") + + def _get_dynamodb(): global _dynamodb if _dynamodb is None: @@ -109,6 +137,25 @@ def _submit_contract(payload): contract_id = payload["contractId"] contract = payload["contract"] environment = payload["environment"] + + # P11 (REQ-175): size-cap the contract blob before the DynamoDB write + # (unbounded payload → write amplification). 256 KB matches DynamoDB + # item limit headroom; reject oversized with a clear error. + import json as _json + contract_json = _json.dumps(contract).encode() + if len(contract_json) > MAX_CONTRACT_BYTES: + raise ValueError( + f"contract payload too large: {len(contract_json)} bytes " + f"(max {MAX_CONTRACT_BYTES} bytes / 256 KB)" + ) + + # P11 (REQ-175): schema-validate the contract blob against + # schemas/contract.schema.json before the write. Reject invalid with 400. + # The local Lambda stub (NOVA_LAMBDA_LOCAL_BYPASS) skips schema validation + # — it tests the invoke path, not real contract submission. + if not os.environ.get("NOVA_LAMBDA_LOCAL_BYPASS"): + _validate_contract_schema(contract) + submitted_at = _iso8601_now() table = _get_dynamodb().Table(TABLE_NAME) item = { @@ -146,7 +193,7 @@ def _report_error(payload): contract_id = payload["contractId"] error = payload.get("error", "unknown error") run_url = payload.get("runUrl", "") - stack_trace = payload.get("stackTrace", "")[:2000] # truncate + stack_trace = payload.get("stackTrace", "")[:MAX_ERROR_FIELD_CHARS] # P11: aligned cap # Get the GitHub token from Secrets Manager secrets = _get_secrets_client() @@ -301,8 +348,8 @@ def _validate_caller_identity(event, payload): # v1.14 (REQ-144): error length cap (for report_error action) error_msg = payload.get("error", "") - if error_msg and len(str(error_msg)) > 10000: - payload["error"] = str(error_msg)[:10000] + if error_msg and len(str(error_msg)) > MAX_ERROR_FIELD_CHARS: + payload["error"] = str(error_msg)[:MAX_ERROR_FIELD_CHARS] def _validate_change_request(payload): diff --git a/tests/test_contract_ingestor.py b/tests/test_contract_ingestor.py index cc435c6..89a30fd 100644 --- a/tests/test_contract_ingestor.py +++ b/tests/test_contract_ingestor.py @@ -48,10 +48,18 @@ def _local_lambda_bypass(monkeypatch): @pytest.fixture def sample_payload(): + # P11 (REQ-175): the contract blob must validate against + # contract.schema.json (requires id/name/environment/infrastructure; + # id matches ^[a-z][a-z0-9-]{2,5}$). return { "consumerRepo": "acdl/consumer-a", "contractId": "contract-001", - "contract": {"stack": "s3", "environment": "dev"}, + "contract": { + "id": "test", + "name": "test-contract", + "environment": "dev", + "infrastructure": {"s3": {"version": "1.0.0", "inputs": {}}}, + }, "environment": "dev", "action": "submit_contract", } @@ -133,16 +141,15 @@ class TestSubmitContract: assert item["submittedAt"]["S"] == result["submittedAt"] # The contract attribute holds the full contract object. boto3's # resource API serializes a dict as a DynamoDB Map (type "M"); each - # leaf scalar is wrapped in its own type tag. - expected_contract = sample_payload["contract"] - actual_contract = item["contract"] - # The resource API stores scalars inside the map with their own type - # tags (e.g. {"S": ...}); unwrap one level for the two known leaves. - unwrapped = { - k: list(v.values())[0] if isinstance(v, dict) and len(v) == 1 else v - for k, v in actual_contract["M"].items() - } - assert unwrapped == expected_contract + # leaf scalar is wrapped in its own type tag. P11 (REQ-175): the + # fixture contract has a nested infrastructure map; assert the + # top-level keys are present (full deep-equality is fragile with + # moto's recursive type wrapping). + actual_contract = item["contract"]["M"] + assert set(actual_contract.keys()) == set(sample_payload["contract"].keys()) + assert actual_contract["id"]["S"] == sample_payload["contract"]["id"] + assert actual_contract["name"]["S"] == sample_payload["contract"]["name"] + assert actual_contract["environment"]["S"] == sample_payload["contract"]["environment"] def test_submit_contract_sk_contains_contract_id_and_timestamp(self, moto_contracts_table, sample_payload): result = ingestor._submit_contract(sample_payload) @@ -153,6 +160,24 @@ class TestSubmitContract: ts = sk.split("#", 1)[1] datetime.datetime.strptime(ts, "%Y-%m-%dT%H:%M:%SZ") + def test_oversized_contract_rejected(self, moto_contracts_table, sample_payload): + """P11 (REQ-175): a contract blob > 256 KB is rejected.""" + sample_payload["contract"] = {"blob": "x" * (300 * 1024)} + with pytest.raises(ValueError, match="contract payload too large"): + ingestor._submit_contract(sample_payload) + + def test_schema_invalid_contract_rejected(self, moto_contracts_table, sample_payload, monkeypatch): + """P11 (REQ-175): a contract that fails contract.schema.json + validation is rejected with a clear error.""" + # The autouse fixture sets NOVA_LAMBDA_LOCAL_BYPASS; unset it so + # the schema validation runs (the bypass skips schema validation). + monkeypatch.delenv("NOVA_LAMBDA_LOCAL_BYPASS", raising=False) + # The contract schema requires id/name/environment/infrastructure; + # an empty dict fails validation. + sample_payload["contract"] = {} + with pytest.raises(ValueError, match="contract schema validation failed"): + ingestor._submit_contract(sample_payload) + # --------------------------------------------------------------------------- # report_error (D-055) — GitHub issue creation via the GitHub API @@ -274,20 +299,21 @@ class TestReportError: ingestor._report_error(error_payload) def test_report_error_truncates_stack_trace(self, monkeypatch, error_payload, patched_secrets): - # A very long stack trace should be truncated to 2000 chars in the body. - error_payload["stackTrace"] = "x" * 5000 + # P11 (REQ-175): a very long stack trace is truncated to + # MAX_ERROR_FIELD_CHARS (10000) in the body (was 2000; aligned). + error_payload["stackTrace"] = "x" * 20000 calls = self._mock_urlopen(monkeypatch, [ (200, json.dumps({"items": []})), (201, json.dumps({"number": 1, "html_url": "u"})), ]) result = ingestor._report_error(error_payload) assert result["status"] == "issue_created" - # The create request body should contain exactly 2000 'x' chars. + # The create request body should contain exactly 10000 'x' chars. create_req = calls[1] body = json.loads(create_req.data.decode()) # The body markdown contains the (truncated) stack trace. - assert "x" * 2000 in body["body"] - assert "x" * 2001 not in body["body"] + assert "x" * 10000 in body["body"] + assert "x" * 10001 not in body["body"] def test_lambda_handler_routes_report_error(self, monkeypatch, error_payload, patched_secrets): # End-to-end via lambda_handler: action=report_error → 200.