andreahlert commented on code in PR #215:
URL: https://github.com/apache/airflow-steward/pull/215#discussion_r3295155084
##########
tools/skill-validator/src/skill_validator/__init__.py:
##########
@@ -131,10 +131,30 @@
PRINCIPLE_CATEGORY = "principle_compliance"
TRIGGER_PRESERVATION_CATEGORY = "trigger_preservation"
BODY_INLINE_CATEGORY = "body_inline"
+PRIVACY_CATEGORY = "privacy"
SOFT_CATEGORIES: frozenset[str] = frozenset(
- {PRINCIPLE_CATEGORY, TRIGGER_PRESERVATION_CATEGORY, BODY_INLINE_CATEGORY},
+ {PRINCIPLE_CATEGORY, TRIGGER_PRESERVATION_CATEGORY, BODY_INLINE_CATEGORY,
PRIVACY_CATEGORY},
)
+# ---------------------------------------------------------------------------
+# Privacy-LLM gate-check constants (write-skill/security-checklist.md §
Pattern 6)
+# ---------------------------------------------------------------------------
+
+# Skill modes that process external / attacker-controlled content.
+_EXTERNAL_CONTENT_MODES: frozenset[str] = frozenset({"Triage", "Mentoring",
"Drafting"})
Review Comment:
The pointer comment helps, but it points at the wrong file:
write-skill/security-checklist.md § Pattern 6 describes the gate-check pattern,
not the mode taxonomy. The canonical mode list actually lives in
[docs/modes.md](https://github.com/docs/modes.md) and currently enumerates
five modes (Triage, Mentoring, Drafting, Pairing, Auto-merge), not the four the
threat-model comment implies.
That matters because Auto-merge is in the taxonomy but missing from
_PRIVACY_EXTERNAL_CONTENT_MODES, and its threat model is the opposite of
Pairing's: no human in the loop. The justification for excluding Pairing
("human remains in the loop") doesn't transfer to Auto-merge, so when the first
Auto-merge skill lands that reads <tracker> bodies, the gate-check will
silently no-op on it. Today there are no
Auto-merge skills (per docs/modes.md:182), so it's a latent gap, not a
live one, but the comment as written would have told future-you the frozenset
is complete when it isn't.
Two concrete adjustments:
1. Repoint the comment to docs/modes.md (the actual canonical source), and
explicitly state Auto-merge's status: either "intentionally excluded because "
or "TODO when the first Auto-merge skill ships".
2. For the longer-term single-source-of-truth: a tiny shared module or a
parser that reads the modes table out of docs/modes.md at validator import time
would close the drift mechanically; the current "I'll
remember to sync" approach has the same failure mode the original review
flagged.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
