diff --git a/AGENTS.md b/AGENTS.md index 688b33035..6ee5727af 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,4 +1,4 @@ # AGENTS.md — ContextualWisdomLab .github -> **Agents: read the master context FIRST.** Before any work, read [`docs/CWL-MASTER-CONTEXT.md`](docs/CWL-MASTER-CONTEXT.md) (mission · naruon-as-platform + inter-component UML · cross-cutting disciplines · conventions · roadmap · current state), the live **GitHub Project #1** (work/roadmap source of truth), the full spec **ContextualWisdomLab/naruon#974**, and operate the Project per [`docs/agent-github-project-protocol.md`](docs/agent-github-project-protocol.md). The repo/Project — not any private agent memory — is the source of truth. +> **Agents: read the master context FIRST.** Before any work, read [`docs/CWL-MASTER-CONTEXT.md`](docs/CWL-MASTER-CONTEXT.md) (mission · naruon-as-platform + inter-component UML · cross-cutting disciplines · conventions · roadmap · current state), [`ARCHITECTURE.md`](ARCHITECTURE.md) (control-plane context and Strix tool-protocol fallback), the live **GitHub Project #1** (work/roadmap source of truth), the full spec **ContextualWisdomLab/naruon#974**, and operate the Project per [`docs/agent-github-project-protocol.md`](docs/agent-github-project-protocol.md). The repo/Project — not any private agent memory — is the source of truth. Strix retries only the exact OpenAI Agents SDK missing-tool signal; it never treats that provider failure as a clean scan. Materialize accepts only exact SHA-256 pins or a bounded relative `-r` include (no `.`/`..`); a lone `--require-hashes` directive is not trust evidence. diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md new file mode 100644 index 000000000..60b21015d --- /dev/null +++ b/ARCHITECTURE.md @@ -0,0 +1,78 @@ +# Architecture — ContextualWisdomLab `.github` + +This repository is the organization control plane. It is not naruon and it +does not own product data. Sibling products remain standalone modules; this +repo publishes org profile assets, reusable required workflows, and the +review/merge schedulers those products consume. + +## System context + +```mermaid +flowchart LR + Buyer["Commercial buyer / reviewer"] + Agents["Agents on AGENTS.md"] + Project["GitHub Project #1"] + Hub["This repo: org .github"] + Products["Owned products
naruon · orchestrator · engines"] + Runner["Required workflows in each repo context"] + + Buyer --> Hub + Agents --> Project + Agents --> Hub + Project --> Hub + Hub --> Runner + Runner --> Products + Products -->|"standalone or as module"| Buyer +``` + +## Strix tool-protocol fallback + +```mermaid +sequenceDiagram + participant Gate as strix_quick_gate + participant Model as Current model + participant Next as Distinct configured model + participant Art as Vulnerability artifacts + + Gate->>Model: scan exact head + alt ModelBehaviorError Tool NAME not found in agent strix + Model-->>Gate: retryable provider protocol failure + Gate->>Next: distinct fallback model + Next-->>Gate: scan evidence + else threshold artifacts already present + Art-->>Gate: fail closed + else fallback exhausted + Gate-->>Gate: fail closed + end +``` + +## Trust boundaries + +- Required review workflows execute **base-branch** scripts. A PR that edits + those workflows cannot widen its own `pull_request_target` token. +- Reviewer agents stay `edit: deny`. They judge; they do not implement. +- Strix retries only the exact OpenAI Agents SDK missing-tool signal for + agent `strix`. The signal is not a clean scan. Severity thresholds, + credentials, and publication stay unchanged. +- Logs and review receipts redact credentials. They do not mask + operational PII that the control plane must process. +- LLM and scheduled agents bind `NVIDIA_NIM_API_KEY` (env may be + `NVIDIA_API_KEY`). They never use `COPILOT_GITHUB_TOKEN`. Existing + review-agent key schemes stay unchanged. + +## Quality gates + +`scripts/ci/` ships with 100% statement/branch coverage and 100% docstrings. +CI installs Python tools only with `pip install --require-hashes`. Contract +tests pin workflow structure and governance prose so drift fails closed. + +## Related durable documents + +- [`docs/CWL-MASTER-CONTEXT.md`](docs/CWL-MASTER-CONTEXT.md) — mission and + ecosystem. +- [`docs/agent-github-project-protocol.md`](docs/agent-github-project-protocol.md) + — Project #1 operation. +- [`PR_GOVERNANCE_AUDIT.md`](PR_GOVERNANCE_AUDIT.md) — live review/merge + contract. +- [`docs/doctoring/strix-tool-protocol-fallback.md`](docs/doctoring/strix-tool-protocol-fallback.md) + — tool-protocol fallback decision and APA 7th citations. diff --git a/CHANGELOG.md b/CHANGELOG.md index bf30091dd..8ebb5c272 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ Semantic Versioning where the repository publishes a release. ### Fixed +- Materialized base Python locks only when every package line is an exact SHA-256 pin or a bounded relative `-r`/`--requirement` include. A lone `--require-hashes` directive, a dotted include such as `./lock.txt`, or `-r other-hashes.txt` no longer enters the trusted build context. +- Retried only the exact OpenAI Agents SDK `ModelBehaviorError: Tool not found in agent strix` signal onto a distinct configured model, without treating that provider failure as a clean scan. - Bounded the Strix quality self-test's deterministic timeout fixtures to 3-second process and 5-second fake-sleep budgets so exact-head policy evidence completes inside the existing job limit without changing production Strix scanner timeouts, providers, credentials, or review semantics. - Allowed commas and ASCII parentheses in the bounded Strix changed-file path policy so legal tracked Packrat fixtures can receive exact-head security analysis, while rejecting raw `..` components before normalization and keeping controls, backslashes, whitespace ambiguity, and shell punctuation fail-closed. - Bound each review-agent invocation key to the wrapper's complete canonical payload, including the base branch and requesting actor; altered fields with a valid-format key now fail before durable-leader election or forwarding, and wrapper write permission is job-scoped. diff --git a/CLAUDE.md b/CLAUDE.md index 1c7bdb2f6..acb151185 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -126,3 +126,7 @@ repeatable compile command. cross-repo references as `owner/repo#num` or full URLs; durable knowledge in the repo/Project, not private memory; one roadmap phase at a time) are defined in `docs/CWL-MASTER-CONTEXT.md` §7 and apply here. +- **Strix tool-protocol fallback** retries only + `agents.exceptions.ModelBehaviorError: Tool not found in agent strix`. + That signal is infrastructure, not a clean scan. See `ARCHITECTURE.md` and + `docs/doctoring/strix-tool-protocol-fallback.md`. diff --git a/docs/doctoring/strix-tool-protocol-fallback.md b/docs/doctoring/strix-tool-protocol-fallback.md new file mode 100644 index 000000000..24bea35ca --- /dev/null +++ b/docs/doctoring/strix-tool-protocol-fallback.md @@ -0,0 +1,30 @@ +# Strix tool-protocol fallback + +## Incident and buyer impact + +Materialize accepts only exact SHA-256 pins or a bounded relative `-r` +include; a lone `--require-hashes` line is not lock evidence. + +Strix can exit before producing security evidence when the selected model +emits a tool call the `strix` agent does not expose: + +`agents.exceptions.ModelBehaviorError: Tool execute not found in agent strix` + +The required check then failed as if the target repository were insecure, +and configured fallback models never ran. + +## Decision + +Classify only that exact exception class, missing-tool phrase, and agent +name `strix` as retryable infrastructure. Fallback to a distinct configured +model. Vulnerability artifacts and fallback exhaustion remain fail-closed. +Generic application errors are not matched. + +## References + +National Institute of Standards and Technology. (2020). *Security and +privacy controls for information systems and organizations* (NIST Special +Publication 800-53 Rev. 5). https://doi.org/10.6028/NIST.SP.800-53r5 + +OpenAI. (2025). *OpenAI Agents SDK*. +https://openai.github.io/openai-agents-python/ diff --git a/scripts/ci/materialize_base_python_requirements.py b/scripts/ci/materialize_base_python_requirements.py index 98cdad459..9848c3ff6 100755 --- a/scripts/ci/materialize_base_python_requirements.py +++ b/scripts/ci/materialize_base_python_requirements.py @@ -87,6 +87,57 @@ def _is_candidate_lock_name(name: str) -> bool: ) +def _is_candidate_lock_path(path: pathlib.PurePosixPath) -> bool: + """Return whether one safe tracked path can name a pip requirements lock. + + In addition to conventional ``requirements*.txt`` names, repositories often + keep concrete environment closures as direct children such as + ``requirements/ci.txt`` or ``service/requirements/package.txt``. Only direct + ``.txt`` children of a directory named ``requirements`` gain this path-based + eligibility; content must still pass the independent complete hash-pin + validation before it reaches the trusted image build context. + """ + return _is_candidate_lock_name(path.name) or ( + path.suffix == ".txt" and path.parent.name == "requirements" + ) + + +def _is_bounded_requirement_include(line: str) -> bool: + """Return whether one requirements include names a bounded relative file. + + Includes are accepted only as a two-token ``-r``/``--requirement`` form + whose target is itself a candidate lock path written as a normalized + relative POSIX path. Absolute paths, ``.`` or ``..`` components, double + slashes, URLs, option-like targets, shell/Windows path separators, + fragments, queries, extra inline options or hashes, and includes of + non-lock files are rejected before a base-owned file can enter the + trusted build context. + The downstream installer still proves that the candidate is an independently + complete hash closure; this predicate grants syntax eligibility only. + """ + fields = line.split() + if len(fields) != 2 or fields[0] not in {"-r", "--requirement"}: + return False + target = fields[1] + if ( + target.startswith(("-", "~")) + or "\\" in target + or ":" in target + or "?" in target + or "#" in target + ): + return False + include_path = pathlib.PurePosixPath(target) + return ( + bool(include_path.parts) + and target == include_path.as_posix() + and not include_path.is_absolute() + and "." not in include_path.parts + and ".." not in include_path.parts + and _is_candidate_lock_path(include_path) + ) + + def _requirement_lines(content: bytes) -> list[str]: """Return logical requirement lines, joining backslash line-continuations. @@ -107,23 +158,26 @@ def _requirement_lines(content: bytes) -> list[str]: def _is_hash_pinned(content: bytes) -> bool: - """Return whether content carries hash pins and is safe to preflight. - - Discovery is content-based rather than name-based so hash-pinned locks in any - location (a service subdirectory, ``requirements-dev.txt``, - ``requirements-test.txt``) can be considered for offline coverage, while an - unpinned or PR-mutable requirements file is still excluded from the networked - build context. Hash syntax cannot prove that a file includes every transitive - dependency, so the trusted image installer separately preflights every - candidate as an independent ``--require-hashes`` closure. An empty file - carries no installable dependency and is not materialized. + """Return whether content carries only trusted pins or bounded includes. + + Discovery is content-based rather than name-based so exact hash-pinned locks + in service subdirectories and role-specific requirements files can be + considered for offline coverage. Candidate syntax is deliberately stricter + than a substring search: each package line must be an exact ``==`` pin with + one or more complete SHA-256 hashes, or a bounded relative requirements + include. A global ``--require-hashes`` directive is not trust evidence by + itself. The downstream installer separately preflights every candidate as an + independent ``pip --require-hashes`` closure, so syntax eligibility never + substitutes for dependency-closure proof. """ lines = _requirement_lines(content) - if not lines: + requirement_lines = [line for line in lines if line != "--require-hashes"] + if not requirement_lines: return False - return any(line == "--require-hashes" for line in lines) or all( - "--hash=" in line or line.startswith(("-r ", "--requirement ")) - for line in lines + return all( + _is_fully_hash_pinned_requirement(line) + or _is_bounded_requirement_include(line) + for line in requirement_lines ) diff --git a/scripts/ci/strix_quick_gate.sh b/scripts/ci/strix_quick_gate.sh index 0f37f3460..e82eabe8d 100755 --- a/scripts/ci/strix_quick_gate.sh +++ b/scripts/ci/strix_quick_gate.sh @@ -2939,6 +2939,15 @@ is_llm_token_limit_error() { return 1 } +# Strix's agent SDK can reject a provider response before it produces scan +# evidence when the model emits a tool call that the selected agent does not +# expose. Treat this exact SDK/provider failure as retryable so a configured +# fallback model can complete the security scan; it is not evidence from the +# target repository and must never be treated as a vulnerability. +is_model_tool_protocol_error() { + grep -Eiq 'agents\.exceptions\.ModelBehaviorError:[[:space:]]*Tool [[:alnum:]_]+ not found in agent strix' "$STRIX_LOG" +} + # Detect whether the strix log contains evidence of infrastructure-level # errors (timeout, rate-limit, transport failures) that indicate the scan # was interrupted or incomplete. Used as a guard to prevent the @@ -2960,6 +2969,10 @@ has_detected_infrastructure_error() { return 0 fi + if is_model_tool_protocol_error; then + return 0 + fi + if is_midstream_fallback_error; then return 0 fi @@ -3838,6 +3851,12 @@ is_model_retryable_error() { return 0 fi + if is_model_tool_protocol_error; then + # A provider/model tool-contract failure is recoverable with a distinct + # configured model, but it is not a clean scan result. + return 0 + fi + if is_timeout_error; then # Process and provider timeouts are not clean evidence, but they are # recoverable across distinct fallback models. Strict provider-signal diff --git a/scripts/ci/test_strix_quick_gate.sh b/scripts/ci/test_strix_quick_gate.sh index 7343c06ac..29a81613f 100755 --- a/scripts/ci/test_strix_quick_gate.sh +++ b/scripts/ci/test_strix_quick_gate.sh @@ -727,6 +727,8 @@ assert_opencode_review_uses_codegraph_and_gpt5_fallback() { assert_file_contains "$REPO_ROOT/scripts/ci/run_opencode_review_model_pool.sh" "skipping remaining attempts for this model" "opencode review skips same-model retries after context-window overflow" assert_file_contains "$REPO_ROOT/.github/workflows/strix.yml" "exceeded your current quota" "strix wrapper neutralizes quota-only provider failures without vulnerability reports" assert_file_contains "$REPO_ROOT/scripts/ci/strix_quick_gate.sh" "billing details" "strix quick gate classifies provider quota starvation as infrastructure" + assert_file_contains "$REPO_ROOT/scripts/ci/strix_quick_gate.sh" "is_model_tool_protocol_error" "strix quick gate retries agent tool-protocol provider failures" + assert_file_contains "$REPO_ROOT/scripts/ci/strix_quick_gate.sh" "ModelBehaviorError:[[:space:]]*Tool [[:alnum:]_]+ not found in agent strix" "strix quick gate identifies unsupported agent tool responses" assert_file_contains "$workflow_file" 'timeout-minutes: 325' "opencode review target contains evidence, the bounded long-review pool, publication, Noema handoff, and cleanup overhead" assert_file_contains "$workflow_file" 'timeout-minutes: 12' "opencode evidence preparation fails closed before it ties up the review queue" assert_file_contains "$workflow_file" 'timeout-minutes: 205' "opencode model pool preserves full-hour candidates within a bounded provider-pool window" @@ -3564,6 +3566,23 @@ REPORT ;; esac ;; + tool-protocol-fallback-success) + case "${STRIX_LLM:-}" in + vertex_ai/tool-protocol-primary) + echo "agents.exceptions.ModelBehaviorError: Tool execute not found in agent strix" + echo "agents.exceptions.ModelBehaviorError: Tool agent_finish not found in agent strix" + exit 1 + ;; + vertex_ai/fallback-one) + echo "scan ok after tool-protocol fallback" + exit 0 + ;; + *) + echo "Error: tool-protocol fallback path unexpected (${STRIX_LLM:-})" >&2 + exit 26 + ;; + esac + ;; openai-primary-quota-fallback-success) case "${STRIX_LLM:-}" in openai/quota-primary) @@ -5825,6 +5844,16 @@ run_filtered_gate_case_if_requested() { input-file-root-override-precedence) run_input_file_root_override_takes_precedence_over_runner_temp_case ;; + tool-protocol-fallback-success) + run_gate_case "tool-protocol-fallback-success" \ + "vertex_ai/tool-protocol-primary" \ + "vertex_ai/fallback-one" \ + "0" \ + "REGEX:Strix quick scan succeeded with fallback model 'vertex_ai/fallback-one' in [0-9]+s\\." \ + "2" \ + "vertex_ai/tool-protocol-primary|vertex_ai/fallback-one" \ + "|" + ;; vertex-without-llm-api-key) run_vertex_without_llm_api_key_case ;; @@ -9329,6 +9358,15 @@ run_gate_case_allow_provider_signal "vertex-primary-midstream-fallback-success" "vertex_ai/midstream-primary|vertex_ai/fallback-one" \ "|" +run_gate_case "tool-protocol-fallback-success" \ + "vertex_ai/tool-protocol-primary" \ + "vertex_ai/fallback-one" \ + "0" \ + "REGEX:Strix quick scan succeeded with fallback model 'vertex_ai/fallback-one' in [0-9]+s\\." \ + "2" \ + "vertex_ai/tool-protocol-primary|vertex_ai/fallback-one" \ + "|" + run_gate_case_allow_provider_signal "vertex-primary-midstream-retry-same-model-success" \ "vertex_ai/retry-midstream-primary" \ "vertex_ai/fallback-one vertex_ai/fallback-two" \ diff --git a/tests/test_materialize_base_python_requirements.py b/tests/test_materialize_base_python_requirements.py index 8a383f0c2..317ab5f5c 100644 --- a/tests/test_materialize_base_python_requirements.py +++ b/tests/test_materialize_base_python_requirements.py @@ -30,6 +30,13 @@ def _created_tool_directory(path: Path) -> str: return str(path) +def _force_linux_x86_64_installer(monkeypatch: pytest.MonkeyPatch) -> None: + """Exercise the installer path that GitHub-hosted linux x86_64 runners use.""" + monkeypatch.setattr(materializer.sys, "platform", "linux") + monkeypatch.setattr(materializer.platform, "machine", lambda: "x86_64") + materializer._install_trusted_uv.cache_clear() + + def test_materializes_only_regular_hash_locks_from_exact_base(tmp_path: Path) -> None: """A PR-modified lock cannot enter the networked coverage image build context.""" repo = tmp_path / "repo" @@ -150,9 +157,24 @@ def test_lock_name_candidates_are_pip_requirements_files() -> None: def test_hash_pin_detection_includes_pinned_and_excludes_unpinned_or_empty() -> None: """Only fully hash-pinned, non-empty lock content is materialized.""" assert not materializer._is_hash_pinned(b"# comment only\n\n") - assert materializer._is_hash_pinned(b"--require-hashes\ndemo==1\n") + assert not materializer._is_hash_pinned(b"--require-hashes\ndemo==1\n") assert materializer._is_hash_pinned(b"demo==1 --hash=sha256:" + b"a" * 64 + b"\n") - assert materializer._is_hash_pinned(b"-r other-hashes.txt\n") + assert materializer._is_hash_pinned(b"-r requirements-other.txt\n") + assert not materializer._is_hash_pinned(b"-r other-hashes.txt\n") + assert not materializer._is_hash_pinned(b"-r ./requirements-other.txt\n") + assert not materializer._is_hash_pinned(b"-r ../escape.txt\n") + assert materializer._is_bounded_requirement_include( + "--requirement requirements-other.txt" + ) + assert not materializer._is_bounded_requirement_include("-r .") + assert not materializer._is_bounded_requirement_include("-r -evil.txt") + assert not materializer._is_bounded_requirement_include("-r ~evil.txt") + assert not materializer._is_bounded_requirement_include("-r C:foo.txt") + assert not materializer._is_bounded_requirement_include("-r foo?bar.txt") + assert not materializer._is_bounded_requirement_include("-r foo#bar.txt") + assert not materializer._is_bounded_requirement_include(r"-r foo\\bar.txt") + assert not materializer._is_bounded_requirement_include("-r") + assert not materializer._is_bounded_requirement_include("-r /abs/requirements.txt") assert not materializer._is_hash_pinned(b"untrusted==1\n") # uv export / pip-compile multi-line continuation format (spec, then --hash= lines). assert materializer._is_hash_pinned( @@ -644,6 +666,7 @@ def test_install_trusted_uv_verifies_version_and_caches_path( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: """The installer writes one executable, verifies its version, and caches it.""" + _force_linux_x86_64_installer(monkeypatch) tool_dir = tmp_path / "uv" monkeypatch.setattr( materializer.tempfile, @@ -690,6 +713,7 @@ def test_install_trusted_uv_rejects_version_process_failures( failure: OSError | subprocess.TimeoutExpired, ) -> None: """A missing or hung downloaded executable is removed and rejected.""" + _force_linux_x86_64_installer(monkeypatch) tool_dir = tmp_path / "uv" monkeypatch.setattr( materializer.tempfile, @@ -721,6 +745,7 @@ def test_install_trusted_uv_rejects_wrong_version_or_exit_status( completed: subprocess.CompletedProcess[bytes], ) -> None: """Unexpected version output or a nonzero status cannot satisfy the pin.""" + _force_linux_x86_64_installer(monkeypatch) tool_dir = tmp_path / f"uv-{completed.returncode}-{len(completed.stdout)}" monkeypatch.setattr( materializer.tempfile,