andreahlert commented on code in PR #215:
URL: https://github.com/apache/airflow-steward/pull/215#discussion_r3272743027
##########
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"})
+
+# The placeholder that marks a skill as referencing the private security
tracker.
+_TRACKER_PLACEHOLDER = "<tracker>"
+
+# Indicates the skill actually *reads* full issue content from the tracker.
+# Skills that only write to / query metadata from the tracker (e.g. create an
+# issue, list milestones) do not pass private content to the model and are
+# therefore exempt from the Privacy-LLM gate-check.
+_TRACKER_READ_PHRASE = "gh issue view"
Review Comment:
literal substring of `gh issue view` is fine for today (audited the tree, no
skill currently reads bodies via REST). but `gh api repos/<tracker>/issues/<N>`
(GET, no `-X PATCH`) also returns the full body and would slip past this check
silently.
not blocking since nothing in main hits that path, but a compound
discriminator (`gh issue view` OR a regex like `gh
api\s+repos/<tracker>/issues/[^\s]+` without `-X`) would close the gap before
the next skill author finds it.
##########
tools/skill-validator/src/skill_validator/__init__.py:
##########
@@ -570,6 +590,61 @@ def validate_principle_compliance(path: Path, text: str)
-> Iterable[Violation]:
)
+# ---------------------------------------------------------------------------
+# Privacy-LLM gate-check (write-skill/security-checklist.md § Pattern 6)
+# ---------------------------------------------------------------------------
+
+
+def validate_privacy_patterns(path: Path, text: str) -> Iterable[Violation]:
+ """Check Privacy-LLM gate-check convention from
``write-skill/security-checklist.md``.
+
+ **Pattern 6** *(SKILL.md only)*: skills whose ``mode`` implies processing
+ external / attacker-controlled content **and** that *read full issue
bodies*
+ from the private ``<tracker>`` repo must invoke the Privacy-LLM gate-check
+ (``privacy-llm-check``) before making any outbound LLM call.
+
+ Three conditions must all be true to trigger the check:
+
+ 1. The file is a ``SKILL.md`` entry point.
+ 2. The ``mode`` frontmatter field is one of the external-content modes
+ (``Triage``, ``Mentoring``, ``Drafting``).
+ 3. The skill both references ``<tracker>`` **and** contains ``gh issue
view``
+ — the command that fetches full issue bodies (embargoed CVE detail,
+ reporter PII, etc.). Skills that only write to / query metadata from
+ the tracker (create an issue, list milestones, search titles) are exempt
+ because they never pass private issue body content to the model.
+
+ All violations are **SOFT** — advisory, surfaced as warnings without
+ failing the run unless ``--strict`` is passed.
+ """
+ # Pattern 6 is only relevant for SKILL.md entry points.
+ if path.name != "SKILL.md":
+ return
+
+ fm = parse_frontmatter(text) or {}
+ mode = fm.get("mode", "")
+ if mode not in _EXTERNAL_CONTENT_MODES:
+ return
+
+ # Only flag skills that both reference the tracker AND read full issue
bodies.
+ if _TRACKER_PLACEHOLDER not in text:
+ return
+ if _TRACKER_READ_PHRASE not in text:
+ return
+
+ if _PRIVACY_LLM_GATE_PHRASE not in text:
Review Comment:
this is a plain `in text` substring check, so any occurrence of the literal
`privacy-llm-check` anywhere in the file satisfies it: inside an HTML comment,
a "don't do this" anti-example, a changelog bullet, or a TODO note. a skill
author can write `<!-- TODO: wire up privacy-llm-check -->` and the validator
passes while the gate is functionally absent.
that converts the check from a guardrail into a tripwire against accidental
omission only, not against intentional or sloppy bypass, which inverts the
threat model for a privacy gate.
minimum fix: require the phrase inside a fenced code block, and ideally
inside a Prerequisites / Step 0 section (you already locate sections
structurally in other validators). at least exclude HTML comments and fenced
"don't" blocks.
##########
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:
`{"Triage", "Mentoring", "Drafting"}` is hardcoded here while the canonical
mode list lives in `write-skill`. when a new mode lands there (e.g. `Coding`,
`Reviewing`), this filter won't extend and the gate-check silently stops firing
on the new mode.
would import from a single source-of-truth, or at least add a comment
pointing at where the canonical list lives so future-you remembers to sync.
##########
tools/skill-validator/src/skill_validator/__init__.py:
##########
@@ -570,6 +590,61 @@ def validate_principle_compliance(path: Path, text: str)
-> Iterable[Violation]:
)
+# ---------------------------------------------------------------------------
+# Privacy-LLM gate-check (write-skill/security-checklist.md § Pattern 6)
+# ---------------------------------------------------------------------------
+
+
+def validate_privacy_patterns(path: Path, text: str) -> Iterable[Violation]:
+ """Check Privacy-LLM gate-check convention from
``write-skill/security-checklist.md``.
+
+ **Pattern 6** *(SKILL.md only)*: skills whose ``mode`` implies processing
+ external / attacker-controlled content **and** that *read full issue
bodies*
+ from the private ``<tracker>`` repo must invoke the Privacy-LLM gate-check
+ (``privacy-llm-check``) before making any outbound LLM call.
+
+ Three conditions must all be true to trigger the check:
+
+ 1. The file is a ``SKILL.md`` entry point.
+ 2. The ``mode`` frontmatter field is one of the external-content modes
+ (``Triage``, ``Mentoring``, ``Drafting``).
+ 3. The skill both references ``<tracker>`` **and** contains ``gh issue
view``
+ — the command that fetches full issue bodies (embargoed CVE detail,
+ reporter PII, etc.). Skills that only write to / query metadata from
+ the tracker (create an issue, list milestones, search titles) are exempt
+ because they never pass private issue body content to the model.
+
+ All violations are **SOFT** — advisory, surfaced as warnings without
+ failing the run unless ``--strict`` is passed.
+ """
+ # Pattern 6 is only relevant for SKILL.md entry points.
+ if path.name != "SKILL.md":
+ return
+
+ fm = parse_frontmatter(text) or {}
+ mode = fm.get("mode", "")
+ if mode not in _EXTERNAL_CONTENT_MODES:
+ return
+
+ # Only flag skills that both reference the tracker AND read full issue
bodies.
+ if _TRACKER_PLACEHOLDER not in text:
+ return
+ if _TRACKER_READ_PHRASE not in text:
Review Comment:
nice call splitting the discriminator into two stages (`<tracker>` AND `gh
issue view`). that's what keeps `security-issue-import-from-pr` correctly
exempt: it references `<tracker>` only as a write destination and never `gh
issue view`s. design holds.
--
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]
