GEN-09: model untrusted artefacts and seed injection canaries - #1340
Conversation
Self-review (FULL security posture)Reviewed exact head Checked factual current-vs-planned wording, all requested threat classes, explicit existing/new/residual columns, #1311/#1312/#1315-#1320 ownership, no claim that injection is solved, fixture manifest/file agreement, malformed/extra-field/vocabulary-escape cases, UTF-8/bounds/canaries, security index links, and the explicit NOT-verified list. Runtime rails remain deliberately deferred and #1323 stays open. Fresh independent FULL review and final-head CI are still required. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the threat model for untrusted artefacts along with a set of hostile test fixtures and a contract test suite to validate them. The feedback focuses on improving the robustness of the contract tests, specifically by ensuring that the source kind assertions can handle duplicate kinds in the future using .Distinct(), and by explicitly validating the format types in the response fixture tests to prevent silent test passes on unexpected formats.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
Merge-blocking CI evidence: CI Extended run https://github.com/Chris0Jeky/Taskdeck/actions/runs/29223718718 completed with |
Chris0Jeky
left a comment
Chris0Jeky
left a comment
There was a problem hiding this comment.
Fresh independent adversarial review — FULL security posture
Reviewed exact head 49c376c6fbb50c91230133891a7da4a97dfbccb0, issue #1323, the PR body/diff, all existing issue comments, review submissions, and both unresolved Gemini threads. I also checked the live #1312 prompt/parser seam and the shipped apply pipeline rather than treating the threat-model claims as self-proving.
All findings below are actionable under the repository's zero-skip review policy.
MEDIUM
-
The threat model overstates the shipped apply-time authorization boundary.
Evidence:docs/security/UNTRUSTED_ARTEFACT_THREAT_MODEL.md:58says the executor “revalidates permission” as an existing control against operation-vocabulary mimicry. The shipped executor callsValidatePermissionsAsyncwith the proposalBoardIdand operations (AutomationExecutorService.cs:137-142), but the policy currently checks only that proposal board plusoperation.TargetIdfor card targets (AutomationPolicyEngine.cs:72-95). The registry then executes mutableparameters.cardId,parameters.boardId, andparameters.columnIdthrough unscoped service overloads. Consequently the effective operation target is not comprehensively bound to the authorized proposal scope. This is the same active repair seam now being handled in PR #1339; it is not yet a shipped control on this PR's base.
Required: narrow the “existing control” wording to what is actually checked, identify the parameter-target binding as an open gate (cross-link #1339/#1319), and do not let the threat model imply that this defense is complete until that repair lands. -
The fixture contract checks that expectations are nonempty, not that the fixtures still represent the declared attacks.
Evidence:UntrustedArtefactFixtureContractTests.cs:41-44only requires each source to contain a generic phrase/canary and nonemptyallowedVerdicts/forbiddenOutcomes; those arrays can be inverted or replaced with unknown vocabulary while the test remains green. At:54-68, JSON response fixtures only need to parse as an object.response-extra-field.jsoncould lose every forbidden extra field, andresponse-vocabulary-escape.jsoncould become{}, without failing the contract. That silently converts future security regressions into benign inputs.
Required: validate the exact allowed/forbidden vocabulary and case-specific semantics (for example manifest-declared required tokens/fields and expected grounded title/evidence), and assert each response fixture contains the condition its ID/disposition claims. -
The documented “uniquely identified, bounded” fixture contract is only enforced for source cases, and unknown formats are accepted.
Evidence:docs/security/UNTRUSTED_ARTEFACT_THREAT_MODEL.md:112describes the fixture files as uniquely identified and bounded. The test makes only source IDs unique (:20) and applies strict UTF-8/size checks only to source files (:35-40). Response IDs/files/canaries are not unique or bounded,response-malformed.txt's canary is not declared/checked, and:65-68treats every format other than literaljsonas “invalid-json.” Thus a typo/new format, duplicate response case, oversized response, or invalid UTF-8 response can remain green. This also confirms the unresolved Gemini format thread is valid.
Required: use one strict bounded UTF-8 loader for every manifest-referenced fixture; enforce unique IDs, paths, and canaries across the relevant sets; explicitly allow onlyjson/invalid-json; and prove the manifest references exactly the intended fixture files (or narrow the documentation claim).
LOW
- The source-kind assertion prevents legitimate suite growth within an existing kind.
Evidence: the unresolved Gemini thread atUntrustedArtefactFixtureContractTests.cs:21-24is correct: adding a second transcript/PDF/image attack case creates a duplicate and failsBeEquivalentTo, even though the coverage requirement is “all three kinds exist,” not “exactly one case per kind forever.”
Required: assert the distinct kind set (while separately enforcing unique case IDs/files/canaries), or explicitly document and defend the exactly-one-case invariant in the thread.
Confirmed coverage / residual risk
- GitHub reports zero closing issue references; #1323 remains open.
- The changed-file set contains only docs, inert text/JSON fixtures, and the fixture-contract test. No #1312 prompt/parser/extractor wiring or other runtime rail was smuggled in.
- Repository search found the canaries referenced only by this fixture directory, its contract test, and the threat-model documentation; they are not executable or runtime-loaded.
- Exact-head local checks passed: fixture contract 3/3, docs governance, link targets, and
git diff --check. The final-head required CI set is also green. - Runtime prompt resistance, strict #1312 containment, extraction budgets, serving headers, UI escaping, consent, and E2E triage remain deliberately unverified/open; that partial scope is documented correctly and is not itself a finding.
Review fix evidence — exact head
|
| Finding | Fix commit(s) | Evidence |
|---|---|---|
| MEDIUM — threat model overstated apply-time effective-target authorization | d28abd04 |
Existing-control wording now names the shipped proposal-board/TargetId limit, marks parameter cardId/boardId/columnId binding as an open gate, and assigns #1319 / PR #1339 in both the matrix and delivery-gate table. |
| MEDIUM — hostile fixture semantics could drift benign or invert | dfa230cd, 64d90e61 |
Response canaries are stable and manifest-declared; the test code independently pins every case's IDs, files, canaries, allowed/forbidden vocabulary, required hostile signals, and JSON attack shape. |
| MEDIUM — response identity/bounds/UTF-8/format coverage was incomplete | dfa230cd, 64d90e61 |
All case IDs/files/canaries are globally unique and bounded; every manifest/fixture payload is bounded strict UTF-8; only exact json/invalid-json formats are accepted; the fixture directory must equal the manifest; JSON and malformed cases take explicit paths. |
| LOW / Gemini — duplicate source kinds blocked suite growth | 64d90e61 |
Source-kind coverage now compares the distinct set while independent identity checks retain unique cases/files/canaries. |
The two Gemini inline threads have commit-specific replies and are resolved.
Final local verification
- Focused
UntrustedArtefactFixtureContractTests: 4 passed - Full
Taskdeck.Application.Tests: 3,221 passed; 0 failed; 0 skipped - Docs governance: passed
- Golden principles: passed
- GitHub operations governance: passed
- Security-doc link targets: resolved
- Runtime fixture/canary reference search: none
git diff --check origin/main...HEAD: passed- GitHub closing issue references after the PR-body update: zero; GEN-09: Untrusted-artefact threat model + prompt-injection rails #1323 remains open
Final-head CI and a fresh exact-head review remain required before this FULL-tier review gate is complete.
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Chris0Jeky
left a comment
Chris0Jeky
left a comment
There was a problem hiding this comment.
Fresh exact-head adversarial re-review — FULL security posture
Reviewed exact head d28abd04021b3fdfa17ef308e08de760172ede37 independently against issue #1323, the complete 600-line final diff, the PR body, all five issue comments (including the final-head Codex no-major-issues comment), all four review submissions, all four inline comments, and both resolved Gemini threads. I also checked the shipped apply policy/handler seam and live PR #1339 rather than relying on the earlier fix summary.
LOW
- The claimed exact manifest/directory agreement ignores nested files.
- Evidence:
backend/tests/Taskdeck.Application.Tests/Services/UntrustedArtefactFixtureContractTests.cs:187-199names the testFixtureDirectory_ShouldContainOnlyManifestReferencedFiles, butDirectory.EnumerateFiles(..., SearchOption.TopDirectoryOnly)only compares direct children. Any file placed underFixtures/untrusted-artefacts/<subdir>/is invisible to the assertion, whiledocs/security/UNTRUSTED_ARTEFACT_THREAT_MODEL.md:113says the manifest and fixture directory agree exactly. That lets an unmanifested fixture sit beside this security suite while the contract and documentation remain green. - Required: either reject all subdirectories explicitly, or enumerate recursively and compare normalized relative paths against the manifest. Keep the manifest path-safety constraint consistent with the chosen model.
- Evidence:
Confirmed at this head
- GitHub reports zero closing issue references; #1323 remains open.
- The changed-file set is limited to security docs, inert text/JSON fixtures, and their test oracle. Repository search found no fixture/canary reference in
backend/src, frontend runtime source, scripts, deploy, or workflows; no prompt rail or runtime execution path was added. - The apply-time authorization wording is now accurate: it names the shipped proposal-board/card-
TargetIdchecks, explicitly leaves effective parameter binding open, and cross-links #1319 / PR #1339 in the matrix and delivery gates. - Case IDs, source kinds, exact allowed/forbidden expectation vocabulary, response dispositions/formats, global manifest ID/file/canary uniqueness, payload size, strict UTF-8 decoding, JSON-vs-invalid-JSON behavior, hostile source signals, and hostile response shapes are independently pinned. The earlier benign/inversion drift paths are closed for the six declared top-level cases.
- Links resolve and the dated doc passes governance. Local exact-head verification passed: fixture contract 4/4, docs governance, golden-principles governance, GitHub-ops governance, and
git diff --check. - Required CI is green except E2E was still running when this review was posted; the separately documented CI Extended startup failure remains tracked in #1330. Runtime prompt resistance, #1312 parser containment, extraction budgets, headers, UI escaping, consent, apply-target repair, and E2E artefact triage remain deliberately open/not verified.
No code or thread state was changed during this review.
Review fix evidence — exact head
|
| Finding | Fix commit | Evidence |
|---|---|---|
| Nested unmanifested fixture files bypassed the claimed exact directory agreement | 1f7d4c99 |
FixtureDirectory_ShouldContainOnlyManifestReferencedFilesAndNoSubdirectories now enumerates every descendant directory with SearchOption.AllDirectories and requires the result to be empty. This matches the existing basename-only manifest rule, so any nested fixture necessarily fails the oracle. |
Verification after the fix:
- Focused
UntrustedArtefactFixtureContractTests: 4 passed; 0 failed; 0 skipped - Docs governance: passed
- Golden-principles governance: passed
- GitHub-operations governance: passed
- Security-document index links: present and target resolves
git diff --checkfor the PR range and repair: passed- Worktree clean after commit; GitHub head is the SHA above
- GitHub closing issue references: zero; GEN-09: Untrusted-artefact threat model + prompt-injection rails #1323 remains open
The finding was posted in a review summary rather than an inline review comment, so GitHub created no resolvable thread node for it. A GraphQL thread audit shows only the two prior Gemini threads, both already resolved; this evidence comment completes the applicable reply/fix mapping.
No production/runtime file, prompt rail, canonical direction doc, or PR body was changed.
|
@codex review Please run a fresh independent FULL security-posture review of exact head |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
CI Extended at exact head |
Summary
Refs #1323
Refs #1311
Deliberate partial scope
Per the overnight coordinator brief, this PR contains the documentation + inert fixtures only. It does not edit PR #1312's shared
LlmCaptureTriagePrompt/parser files, wire runtime prompt rails, add extraction budgets, change egress consent, implement GEN-06 rendering, or change the proposal executor. The threat model marks those as open delivery gates and the issue remains open.The prompt/extractor follow-up must bind these fixtures and prove hostile content produces only grounded task candidates or an empty verdict, while malformed/extra-field/vocabulary-escape responses take deterministic fallback. Until then, the PR explicitly says prompt-injection behavior is not verified.
Apply-time binding of effective
cardId/boardId/columnIdparameters to the authorized proposal scope is also an open gate owned by #1319 / PR #1339; this PR documents that boundary without changing runtime authorization.Verification
UntrustedArtefactFixtureContractTests: 4 passed.Taskdeck.Application.Tests: 3,221 passed, 0 failed, 0 skipped.node scripts/check-docs-governance.mjs: passed.node scripts/check-golden-principles.mjs: passed.node scripts/check-github-ops-governance.mjs: passed.git diff --check origin/main...HEAD: passed.Docs impact
docs/security/UNTRUSTED_ARTEFACT_THREAT_MODEL.md.docs/security/README.mdand rootSECURITY.md.Not verified / residual risk