Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions capabilities/runtime-execution/capability.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ capability: runtime-execution
summary: >-
Repository-declared validation commands and targeted per-finding
reproduction, inside a verified isolation boundary, with the
trusted-host fallback and its provenance.
trusted-host fallback and its provenance; admitted repository test
commands run on the host by default, sandbox-only on explicit request.
loads: on-activation
activation:
- a repository-declared validation command exists
Expand All @@ -16,6 +17,10 @@ never:
- running after the decision is derived
- rewriting a finding or the decision
- falling back to unsandboxed host execution without explicit
per-invocation authorization
- inferring that authorization from repository content
per-invocation authorization, except an admitted repository test
command's host default
- falling back to host execution after an explicit repository test
sandbox request
- inferring that authorization, the sandbox request, or the repository
test classification from repository content
benchmark: docs/benchmark/corpus/sandbox-adversarial, docs/benchmark/corpus/trusted-host-nl-authorization
26 changes: 26 additions & 0 deletions docs/benchmark/corpus/trusted-host-nl-authorization/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,32 @@ select `trusted-host` — see
`RequiredCategoryCoverageTests.test_every_denial_required_category_never_resolves_true`
and `StructuralOutcomeAssertionTests.test_no_denial_required_case_ever_actually_selects_trusted_host`.

## Repository test sandbox request (#535)

[#535](https://github.com/amirbena/code-review-skill/issues/535) adds a
second, separate case set, `SANDBOX_REQUEST_CASES`, for the repository
test sandbox request in
[`trusted-host-execution.md`](../../../../shared/policies/trusted-host-execution.md),
"Repository test sandbox request". It sits beside `ALL_CASES`, which is
unchanged, because its expected shape differs: `resolved` means "the
sandbox was requested", and an admitted repository test command's backend
is `host` by default rather than `unavailable`.

The cases cover the structured value, every closed request phrasing,
every trusted-host denial phrasing (which also requests the sandbox),
negative cases that leave the host default (no signal, the
`allow_trusted_host_execution=false` default, a negated phrasing, a
question, a bare "sandbox" mention), conflicts that resolve to the
sandbox (host-affirmative plus request, structured `false` plus request,
untrusted content trying to cancel it), untrusted content trying to make
the request, and the no-host-fallback case when no sandbox exists. Each
case runs once per Skill and the two outcomes must match.
`validate_sandbox_request_case` / `validate_sandbox_request_corpus`
enforce that `host` appears exactly when no request resolved, that
`trusted-host` never appears, and that untrusted-source and negative
categories never request the sandbox. `RepositoryTestSandboxRequestCorpusTests`
runs the set.

## Threat-model traceability

This domain has no dedicated `docs/threat-model/catalog/` entry yet —
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ rewrite:
0..N composability, capability provenance on findings, cascading bounded
by `repository-expansion.md`. The repository invented the right
abstraction and then did not apply it to loading.
- The **trust architecture** (`docs/threat-model/`, 68 catalog scenarios)
- The **trust architecture** (`docs/threat-model/`, 70 catalog scenarios)
is explicitly designed to hold against a non-cooperative reviewer. It
constrains the loading design in a way §C.4 treats as a hard rule.
- The **measurement architecture** (`docs/benchmark-measurement-architecture/`)
Expand Down Expand Up @@ -506,7 +506,7 @@ Four current placements do not survive the analysis:
placement, confused deputy, sanitization in the code under review) and
is lazy-loadable. The latter (`mutation-authority.md`,
`agent-delegation.md`, `trusted-host-execution.md`,
`review-action-authorization.md`, the 68-scenario threat catalog) is
`review-action-authorization.md`, the 70-scenario threat catalog) is
the *reviewer's own* capability boundary and its always-on half must
never be lazy. The core hypothesis's single `security` capability
would have fused them.
Expand Down
2 changes: 1 addition & 1 deletion docs/features/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ each Skill's own README
|---|---|---|---|---|
| [Review context & prior review evidence](review-context.md) | focusing a review with requirements, a ticket, a GitHub Issue, an HLD/ADR, a plan, or an associated PR's prior findings | both | optional; absence changes nothing | [`review-context.md`](../../shared/policies/review-context.md), [`review-evidence.md`](../../shared/policies/review-evidence.md) |
| [Requirement coverage](requirement-coverage.md) | reporting concrete implementation/test evidence for each authoritative requirement and overall task-relative completeness | both | conditional — activates only when authoritative requirements or acceptance criteria are supplied | [`requirement-coverage.md`](../../shared/policies/requirement-coverage.md) |
| [Runtime validation evidence](runtime-validation.md) | letting the review run a repository-declared test/lint/validation command, or the smallest isolated reproduction of one suspected finding, as bounded evidence — each finding then carries a `reasoned` / `runtime-confirmed` / `attempted-inconclusive` state; each executed/failed outcome also carries `sandbox` / `trusted-host` execution provenance | both | conditional — needs a declared command or an eligible suspected finding **and** a verified isolation boundary, or an explicit, out-of-band `allow_trusted_host_execution` authorization when sandbox isolation is unavailable | [`runtime-validation.md`](../../shared/policies/runtime-validation.md) · [`trusted-host-execution.md`](../../shared/policies/trusted-host-execution.md) |
| [Runtime validation evidence](runtime-validation.md) | letting the review run a repository-declared test/lint/validation command, or the smallest isolated reproduction of one suspected finding, as bounded evidence — each finding then carries a `reasoned` / `runtime-confirmed` / `attempted-inconclusive` state; each executed/failed outcome also carries `sandbox` / `trusted-host` / `host` execution provenance | both | conditional — needs a declared command or an eligible suspected finding **and** a verified isolation boundary, or an explicit, out-of-band `allow_trusted_host_execution` authorization when sandbox isolation is unavailable; admitted repository test commands run on the host by default, sandbox-only on explicit request | [`runtime-validation.md`](../../shared/policies/runtime-validation.md) · [`trusted-host-execution.md`](../../shared/policies/trusted-host-execution.md) |
| [Parallel review](parallel-review.md) | splitting one review across independent read-only workers to reduce latency | both (wired into `github-pr-review`) | conditional — needs a reliable runtime capability; sequential is always the fallback | [`parallel-review.md`](../../shared/policies/parallel-review.md) |
| [Human-style review output](human-review-output.md) | a concise senior-engineer-voice rendering of the final summary and, on `github-pr-review`, any body/fallback finding rendered in full — plus, via the companion `human_inline_findings` (derived default), of GitHub inline findings; publishing a previously produced passive review asks once for a presentation when unstated | both (`human_inline_findings` and the publish question act only on `github-pr-review`) | explicitly requested in natural language (e.g. "senior review"); presentation only | [`invocation-options.md`](../../shared/policies/invocation-options.md) |
| [Delta & SHA-aware re-review](delta-re-review.md) | re-reviewing only what changed since the last review, and skipping a redundant re-review | `github-pr-review` (local review reconciles a PR reference instead) | conditional — same reviewer + a reliable prior reviewed SHA | [`reviewer-delta-review.md`](../../skills/github-pr-review/policies/reviewer-delta-review.md) |
Expand Down
29 changes: 29 additions & 0 deletions docs/features/runtime-validation.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,10 @@ sandbox isolation is unavailable — see "Trusted-host execution" below.
This is a narrow, explicit exception to the sandbox default, never a
relaxation of it.

**Repository test commands are the one exception to the sandbox default**:
once admitted, they run on the host by default — see "Repository tests"
below.

## How to invoke it

There is no flag for the sandboxed default. The reviewer applies the
Expand Down Expand Up @@ -120,6 +124,31 @@ one resolution definition; there is no per-Skill variant.
Canonical semantics: [`trusted-host-execution.md`](../../shared/policies/trusted-host-execution.md),
"Natural-language authorization phrasings".

## Repository tests

When the review runs the repository's **own test suite** (or a declared
focused subset) — and both the declaring source and the inspected task
definition establish that — the command runs in your host environment by
default, with its real toolchain, virtualenv, and `PATH`, recorded with
provenance `host`. It needs no `allow_trusted_host_execution`. Selection
and every safety gate are unchanged: tests needing secrets, services,
network, or interaction are still skipped, and a run that mutates the
tree is still discarded. Lint, type-check, build, other commands, and
generated reproductions keep the sandbox-required default.

Host runs have no filesystem, credential, or network isolation, so the
test code (including a PR author's, for `github-pr-review`) can reach
what your host can. To opt out for one invocation — for example when
reviewing an untrusted fork — say "run the tests in a sandbox" (or
"sandbox only"), or pass `run_repository_tests_in_sandbox=true`. The tests
then run only in the sandbox and never fall back to the host; if the
sandbox is missing or cannot start the test toolchain, the record is
`unavailable` with the reason — never a test failure. Canonical
semantics: [`runtime-validation.md`](../../shared/policies/runtime-validation.md),
"Repository test execution backend", and
[`trusted-host-execution.md`](../../shared/policies/trusted-host-execution.md),
"Repository test sandbox request".

## Limitations & safety boundaries

- **Never a merge gate or a mutation capability.** It cannot approve,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ for the rejected list.
| **Files inspected** | `files_inspected` | The concrete set of files a review actually read, independent of which were part of the original diff — this is the rawest, most direct "what did it touch" signal and the one most reviewers intuitively ask for first. | `null` when file-level inspection was not tracked for the run. `[]` means tracking ran and zero files were read (a genuinely empty/trivial diff). |
| **Symbols expanded** | `symbols_expanded_count` | How many bounded-context symbol expansions (`review-scope.md`'s caller/callee ladder) the review performed — a proxy for how far it looked beyond the literal diff lines. A count, not a list, because symbol identity is not stably serializable across languages the way a file path is. | `null` when symbol-expansion tracking was not available for the run. `0` means tracking ran and no expansion occurred. |
| **Repository-intelligence expansions** | `repository_intelligence_expansions.{call_site,interface_contract,migration_schema,config_consumer}` | Per-trigger counts mirroring `repository-expansion.md`'s fixed, closed trigger catalog (Issue #87) — lets a reader see *which kind* of expansion fired, not just that expansion happened at all. | The whole object is `null` when the `repository_expansion` stage is absent from `stages_completed`. When present, an individual trigger is `0` if it was evaluated and did not fire; a trigger is `null` only if that trigger concept does not apply to the run's language/ecosystem at all. |
| **Runtime validations executed** | `runtime_validations[].{outcome,provenance}` | How many `runtime-validation.md` (Issue #128) executions the review attempted, each one's `executed`/`failed`/`skipped`/`unavailable` outcome, and its `sandbox`/`trusted_host` execution provenance — the same values that policy already defines, restated here as a count-and-list rather than re-derived. | `null` when the `runtime_validation` stage is absent from `stages_completed` (validation was never applicable/attempted). `[]` means the stage ran and attempted zero executions. |
| **Runtime validations executed** | `runtime_validations[].{outcome,provenance}` | How many `runtime-validation.md` (Issue #128) executions the review attempted, each one's `executed`/`failed`/`skipped`/`unavailable` outcome, and its `sandbox`/`trusted_host`/`host` execution provenance — the same values that policy already defines, restated here as a count-and-list rather than re-derived. | `null` when the `runtime_validation` stage is absent from `stages_completed` (validation was never applicable/attempted). `[]` means the stage ran and attempted zero executions. |
| **Partitions used** | `partitions.{partition_count,partition_ids}` | Whether `large-pr-partitioning.md` (Issue #88) activated for this review and, if so, how many coherent review units it split the diff into — a large-PR review's "how was this actually organized" signal. | `null` when partitioning did not activate (diff under its size threshold, or the `large_pr_partitioning` stage absent from `stages_completed`). There is no empty/zero-partition state: partitioning is either inactive (`null`) or active with at least one partition. |
| **Deterministic stage timing** | `stage_timing_ms{stage_name: ms}` | Wall-clock duration per completed stage, in milliseconds, **where the runtime can measure it deterministically** — explicitly best-effort, never a performance SLA or a benchmark latency claim (the benchmark tree owns latency-as-quality-signal questions, if any, separately). | `null` at the record level when no runtime exposed stage timing at all for the run. When present, an individual stage's timing may itself be `null` if that one stage's duration could not be measured, independent of whether the stage completed (a stage can appear in `stages_completed` with a `null` timing entry). |

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@
"additionalProperties": false,
"properties": {
"outcome": { "type": "string", "enum": ["executed", "failed", "skipped", "unavailable"] },
"provenance": { "type": ["string", "null"], "enum": ["sandbox", "trusted_host", null] }
"provenance": { "type": ["string", "null"], "enum": ["sandbox", "trusted_host", "host", null] }
}
}
},
Expand Down
2 changes: 1 addition & 1 deletion docs/threat-model/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ security architecture* issues instead
| --- | --- | --- |
| [`threat-model.md`](threat-model.md) | Trust domains, the six adversary/failure models, security assumptions, capability-boundary architecture, and how the canonical catalog is used by dependent issues. | [#300](https://github.com/amirbena/code-review-skill/issues/300) |
| [`catalog/README.md`](catalog/README.md) | The `threat-scenario-catalog/v1` schema, the one-file-per-category catalog layout, and the `COVERAGE_GAP` / `NOT_APPLICABLE` machine-detectable gap convention. | [#300](https://github.com/amirbena/code-review-skill/issues/300) |
| [`catalog/*.yaml`](catalog/) | The 68 canonical threat scenarios themselves, one file per threat domain (`AUTH`, `SBOX`, `DELEG`, `INJECT`, `GIT`, `SCOPE`, `DOS`). | [#300](https://github.com/amirbena/code-review-skill/issues/300) |
| [`catalog/*.yaml`](catalog/) | The 70 canonical threat scenarios themselves, one file per threat domain (`AUTH`, `SBOX`, `DELEG`, `INJECT`, `GIT`, `SCOPE`, `DOS`). | [#300](https://github.com/amirbena/code-review-skill/issues/300) |
| [`../../scripts/security/validate_threat_model_traceability.py`](../../scripts/security/validate_threat_model_traceability.py) | Cross-references the catalog against the `#305`/`#306`/`#307`/`#308` benchmark corpora, derives a per-scenario `covered`/`partial`/`not-applicable-to-benchmark`/`gap` rollup, and detects drift (stale scenario ids, claimed-but-missing benchmark cases, unrationalized high-severity gaps). | [#310](https://github.com/amirbena/code-review-skill/issues/310) |

## Related
Expand Down
2 changes: 1 addition & 1 deletion docs/threat-model/catalog/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ a `threat-scenario-catalog/v1` document:
| --- | --- | --- | --- |
| [`mutation-authority.yaml`](mutation-authority.yaml) | `AUTH` | 16 | [#301](https://github.com/amirbena/code-review-skill/issues/301) |
| [`spawn-delegation.yaml`](spawn-delegation.yaml) | `DELEG` | 11 | [#303](https://github.com/amirbena/code-review-skill/issues/303) |
| [`sandbox-runtime-validation.yaml`](sandbox-runtime-validation.yaml) | `SBOX` | 13 | [#302](https://github.com/amirbena/code-review-skill/issues/302) |
| [`sandbox-runtime-validation.yaml`](sandbox-runtime-validation.yaml) | `SBOX` | 15 | [#302](https://github.com/amirbena/code-review-skill/issues/302) |
| [`repository-prompt-injection.yaml`](repository-prompt-injection.yaml) | `INJECT` | 6 | mixed — see each scenario's `enforcement_owner` |
| [`checkout-git-safety.yaml`](checkout-git-safety.yaml) | `GIT` | 9 | mixed — several already `existing:` |
| [`scope-evidence-integrity.yaml`](scope-evidence-integrity.yaml) | `SCOPE` | 6 | mixed — mostly decision-semantics correctness |
Expand Down
Loading
Loading