diff --git a/docs/architecture/security-model.md b/docs/architecture/security-model.md index 6dfae53ec..28036372c 100644 --- a/docs/architecture/security-model.md +++ b/docs/architecture/security-model.md @@ -2,13 +2,13 @@ Generated files are updated through authoritative inputs or the compiler and regeneration; handwritten edits to them are forbidden. Code review enforces this -ownership rule. This `next` revision still contains the legacy budget/reporting -workflows and publisher code pending separate canonical regeneration. This -change does not remove or disable those workflows. Until removal, preserve and -review their trusted-code, artifact-validation, event/repository/revision, -freshness, and least-privilege boundaries. Their historical protocol is documented -in `scripts/castiron/CUSTOM_CODE.md`; it does not authorize new generated-file -patches or a replacement budget system. +ownership rule. Canonical regeneration retires the legacy budget/reporting +workflows and publisher. Determine whether the publisher boundary below applies +by checking the files in the revision being reviewed; do not infer retirement +from this documentation change. After canonical removal, do not recreate the +tooling or introduce a replacement budget system. +Repository rules and hosted workflow settings are separate: inspect their actual +state and required checks rather than assuming source-file removal changes them. This is the repository's single canonical detailed threat model. Use it for security review and Codex Security scans of the revision being reviewed. @@ -116,18 +116,31 @@ not exposed to the attestation action. boundary where independently mutable data enters a parser/evaluator, where runtime/network data reaches a sensitive sink, or where PR-controlled code can reach protected CI or release credentials. -- Candidate Git objects can still be lower-trust data when a separately - privileged workflow evaluates them. The workflow_run custom-code workflow - checks out trusted main, computes from current PR Git objects, and freshness - checks the exact head before publishing status; breaking that evaluator boundary - remains a security finding. (.github/workflows/castiron-custom-code-comment.yml:34-57; - .github/workflows/castiron-custom-code-comment.yml:74-98; - .github/workflows/castiron-custom-code-comment.yml:128-181) - Merged, reviewed repository source intentionally executes with trusted build/release authority. The security boundary is externally enforced branch governance, CODEOWNERS/rulesets, verified release selection, and protected environments—not an imagined sandbox around checked-in source. +### Legacy publisher boundary — only while present + +If `.github/workflows/castiron-custom-code-comment.yml` and its +`scripts/castiron` publisher exist in the reviewed revision, they remain an +active privileged boundary. The `workflow_run` job evaluates candidate Git +objects using a trusted checkout and publishes statuses with `statuses: write`. +A PR author controls those candidate objects but must not gain execution with +that publishing authority or forge an authoritative result. Preserve trusted-code +selection, event/repository/revision binding, artifact validation, exact-head +freshness checks, and least-privilege permissions. Review their implementation +in that revision, including the protocol in `scripts/castiron/CUSTOM_CODE.md`. + +The review hypothesis is that mutable PR Git objects or artifacts could be +accepted as trusted code or as an unbound status result. Such a bypass crosses a +privilege boundary beyond ordinary PR CI and remains reportable while this +publisher exists. Once canonical regeneration deletes the workflow and publisher, +this component and hypothesis no longer apply; the runtime, ordinary CI, cache, +and release boundaries elsewhere in this model still apply. Hosted rules must +still be verified separately. + ### Gradle cache accepted risk and assumptions The Kotlin Gradle plugin used by this repository is affected by @@ -181,7 +194,6 @@ These are review hypotheses, not confirmed findings. | High | Untrusted webhook bytes are parsed or acted on without valid signature/freshness checks. | Embedding application exposes webhook ingress and relies on SDK verification. | Forged event acceptance. | Required headers, bounded timestamp tolerance, HMAC-SHA256, timing-safe compare, then parse; no replay deduplication. | Preserve verify-before-parse ordering and focused regression tests; embedding applications must deduplicate accepted webhook IDs when replay matters. | openai-java-core/src/main/kotlin/com/openai/services/WebhookVerification.kt:19-99; openai-java-core/src/main/kotlin/com/openai/services/blocking/WebhookServiceImpl.kt:33-41 | | Medium | Independently mutable API/SSE JSON crosses a parser boundary and causes unsafe interpretation, resource exhaustion, or sensitive exception/log output. | Attacker controls an API/network response or configured proxy; sink is reachable in caller deployment. | Client failure, data exposure, or downstream misuse. | Strict mapper defaults, typed parsing, stream iteration, and explicit DEBUG diagnostics. | Keep parsing robust and large-payload-compatible; avoid treating model output as executable authority without caller controls. | openai-java-core/src/main/kotlin/com/openai/core/ObjectMappers.kt:34-116; openai-java-core/src/main/kotlin/com/openai/core/handlers/SseHandler.kt:19-60; AGENTS.md:61-78 | | Medium | Credentials or customer payloads reach an unintended destination or uncontrolled log. | Lower-trust actor independently controls base URL/proxy/TLS/log sink, or diagnostics are enabled in an unsafe deployment. | Secret or customer-data disclosure. | Production URL default, caller-owned configuration, auth wrappers, and default sensitive-header redaction; DEBUG is explicit. | Preserve redaction and destination/TLS controls; document caller obligations. | openai-java-core/src/main/kotlin/com/openai/core/ClientOptions.kt:151-172; openai-java-core/src/main/kotlin/com/openai/core/ClientOptions.kt:601-665; openai-java-core/src/main/kotlin/com/openai/core/http/LoggingHttpClient.kt:24-48; openai-java-core/src/main/kotlin/com/openai/core/http/LoggingHttpClient.kt:91-142 | -| Medium | A separately privileged evaluator consumes mutable PR Git objects, artifacts, or cache entries as trusted code/data. | Privileged workflow reads candidate state without binding/ref validation. | Privilege crossing beyond ordinary PR CI. | Custom-code workflow checks out main, uses current PR objects, and freshness-checks exact head; PR cache scope remains isolated. | Preserve trusted checkout and exact-head/digest binding. | .github/workflows/castiron-custom-code-comment.yml:34-57; .github/workflows/castiron-custom-code-comment.yml:128-181; .github/workflows/ci.yml:120-129 | | Low / not a boundary by itself | A PR changes a checked-in example, test, fixture, build script, or generated source and ordinary PR CI executes it. | The same PR author already controls the tracked executable checkout. | No new capability beyond the unprivileged PR job. | PR CI has read-only contents and no release credentials; cache scope is enforced outside PR code. | Investigate only if execution crosses an independent input or protected-authority boundary. | .github/workflows/ci.yml:8-20; .github/workflows/ci.yml:33-48; .github/workflows/ci.yml:92-107 | ## Severity calibration (critical, high, medium, low)