docs(#1807): update THREAT_MODEL.md for single-source trusted task architecture - #1810
docs(#1807): update THREAT_MODEL.md for single-source trusted task architecture#1810fullsend-ai-coder[bot] wants to merge 1 commit into
Conversation
…chitecture PR #1806 removed the data.trusted_task_rules input path from lib/tekton/trusted.rego, eliminating the dual-source merge that concatenated system-level rules with ruleData-level rules. THREAT_MODEL.md still described the removed architecture in multiple sections. Updated sections: - Section 3.3: rewritten to describe single-source flow through lib_rule_data("trusted_task_rules") with the standard 4-level precedence cascade - Key table (line 135): clarified both trusted task keys flow exclusively through rule_data - Threat DP-2: updated to reflect narrower injection surface through rule data precedence cascade - Threat LE-4: removed stale concatenation-order concern - Open question #3: reframed from merge-precedence to pattern-specificity interactions - Recommendations #3 and #6: updated references to reflect single-source architecture Closes #1807
|
🤖 Finished Review · ✅ Success · Started 1:54 PM UTC · Completed 2:08 PM UTC Commit: |
Codecov Report✅ All modified and coverable lines are covered by tests.
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
ReviewFindingsHigh
Low
Labels: PR modifies only THREAT_MODEL.md — a documentation-only change Next steps:
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
| | Key | Effect if weakened | | ||
| |-----|---------------------| | ||
| | `trusted_tasks` / `trusted_task_rules` | Malicious task bundles treated as trusted | | ||
| | `trusted_tasks` / `trusted_task_rules` | Malicious task bundles treated as trusted (both flow exclusively through `rule_data`) | |
There was a problem hiding this comment.
[high] Technical accuracy in documentation
The parenthetical '(both flow exclusively through rule_data)' is factually incorrect for trusted_tasks. In policy/lib/tekton/trusted.rego, _trusted_tasks_data is defined as object.union(data.trusted_tasks, lib_rule_data("trusted_tasks")), meaning the legacy trusted_tasks key still has a direct data.trusted_tasks input path that is merged with the rule_data path. Only trusted_task_rules flows exclusively through lib_rule_data. This error misrepresents the trust boundary for the legacy system in a security-critical document.
Suggested fix: Change the parenthetical to clarify that only trusted_task_rules flows exclusively through rule_data, while trusted_tasks still merges data.trusted_tasks with lib_rule_data("trusted_tasks").
| `quay.io/konflux-ci/tekton-catalog/data-acceptable-bundles`), merged with | ||
| ruleData-provided `trusted_tasks` / `trusted_task_rules`. | ||
| **Source**: `trusted_tasks` and `trusted_task_rules` flow exclusively through | ||
| `lib_rule_data` (see section 3.2 for the 4-level precedence cascade). There is |
There was a problem hiding this comment.
[high] Technical accuracy in documentation
Section 3.3 states that trusted_tasks and trusted_task_rules flow exclusively through lib_rule_data with no separate direct data.* input path. This is incorrect for trusted_tasks. The code shows _trusted_tasks_data := object.union(data.trusted_tasks, lib_rule_data("trusted_tasks")), meaning data.trusted_tasks is still a direct input path. This overgeneralization in a threat model conceals a real input path that an attacker could potentially influence.
Suggested fix: Rewrite section 3.3 to distinguish between the two systems: trusted_task_rules flows exclusively through lib_rule_data, while trusted_tasks (legacy) is sourced from both data.trusted_tasks and lib_rule_data("trusted_tasks") merged via object.union.
| | `pipeline_intention` | Operational mode of certain rules changed | | ||
|
|
||
| ### 3.3 Trusted task data (`data.trusted_tasks`, `data.trusted_task_rules`) | ||
| ### 3.3 Trusted task data |
There was a problem hiding this comment.
[low] Internal consistency
Section heading changed to remove data.trusted_tasks reference, but data.trusted_tasks is still a valid input path in the code. Fixing the high-severity body text errors would naturally lead to restoring this in the heading.
fullsend-ai-review
Bot
added
the
documentation
Summary
data.trusted_task_rulesinput path (PR feat(EC-2031): remove data.trusted_task_rules in favor of rule_data only #1806)trusted_task_rulesflows exclusively throughlib_rule_datawith the standard 4-level precedence cascadeContext
PR #1806 removed the
data.trusted_task_rulesdirect input path frompolicy/lib/tekton/trusted.rego, consolidating all trusted task rule data throughlib_rule_data("trusted_task_rules"). The THREAT_MODEL.md was not updated in that PR, leaving 6+ sections describing a trust boundary architecture that no longer exists.Validation
grep -c 'data\.trusted_task_rules' THREAT_MODEL.mdreturns 0grep -q 'lib_rule_data' THREAT_MODEL.mdfinds references to the new architecturegrep -c 'array\.concat' THREAT_MODEL.mdreturns 0Closes #1807
Post-script verification
agent/1807-update-stale-threat-model)4b089513609ae33616c01509cb0a0ccc5db4fb11..HEAD)