From db36317dd019d3ddd4d238bcd23d14bfbdb12b52 Mon Sep 17 00:00:00 2001 From: Justin McLean Date: Sun, 24 May 2026 14:56:20 +1000 Subject: [PATCH] Flag unbounded gh list calls Generated-by: Codex (GPT-5) --- .../pr-management-code-review/selectors.md | 6 +- .../skills/pr-management-triage/actions.md | 3 + .../skills/security-issue-invalidate/SKILL.md | 3 + .../security-tracker-stats-dashboard/SKILL.md | 12 +--- .../src/skill_validator/__init__.py | 43 ++++++++++++++ tools/skill-validator/tests/test_validator.py | 58 +++++++++++++++++++ 6 files changed, 111 insertions(+), 14 deletions(-) diff --git a/.claude/skills/pr-management-code-review/selectors.md b/.claude/skills/pr-management-code-review/selectors.md index de191545..bbd810bc 100644 --- a/.claude/skills/pr-management-code-review/selectors.md +++ b/.claude/skills/pr-management-code-review/selectors.md @@ -117,7 +117,7 @@ since="${SINCE:-30 days ago}" # default; overridable via since: # 1) Files in open PRs authored by viewer: viewer_open_prs=$(gh pr list --repo --author "$viewer" \ - --state open --json number --jq '.[].number') + --state open --limit 100 --json number --jq '.[].number') mine_via_open_prs=$(for n in $viewer_open_prs; do gh pr view "$n" --repo --json files --jq '.files[].path' @@ -490,8 +490,8 @@ gh pr list \ ``` Often combined with `area:` to scope. Without `area:` it's -typically too broad for a single sitting; warn the maintainer if -the result count exceeds 30. +typically too broad for a single sitting. If the result count equals +the `--limit` value, note that there may be additional results not shown. --- diff --git a/.claude/skills/pr-management-triage/actions.md b/.claude/skills/pr-management-triage/actions.md index 9a1fc76e..0fb96714 100644 --- a/.claude/skills/pr-management-triage/actions.md +++ b/.claude/skills/pr-management-triage/actions.md @@ -620,6 +620,7 @@ path. ```bash # 1. List open PRs by the author gh pr list --repo --author --state open \ + --limit 100 \ --json number --jq '.[].number' # 2. For each PR, in parallel — close + label + comment @@ -630,6 +631,8 @@ for pr in $PR_NUMBERS; do done ``` +If the result count equals the limit, note that there may be additional results not shown. + Body template: [`comment-templates.md#suspicious-changes`](comment-templates.md). The comment is deliberately short and non-accusatory — the diff --git a/.claude/skills/security-issue-invalidate/SKILL.md b/.claude/skills/security-issue-invalidate/SKILL.md index 085ed618..420f0712 100644 --- a/.claude/skills/security-issue-invalidate/SKILL.md +++ b/.claude/skills/security-issue-invalidate/SKILL.md @@ -244,12 +244,15 @@ in Gmail. ```bash # Find open trackers with a INVALID triage proposal gh issue list --repo --state open --label "needs triage" \ + --limit 100 \ --json number,title,comments \ --jq '.[] | select(.comments | map(.body) | any( startswith("**Triage proposal**") and contains("INVALID") )) | .number' ``` +If the result count equals the limit, note that there may be additional results not shown. + Then, per resolved tracker, check the triage-proposal comment's reactions and follow-up comments for the team-consensus marker via `gh api repos//issues/comments//reactions`. diff --git a/.claude/skills/security-tracker-stats-dashboard/SKILL.md b/.claude/skills/security-tracker-stats-dashboard/SKILL.md index d940c0b9..f7b4eee4 100644 --- a/.claude/skills/security-tracker-stats-dashboard/SKILL.md +++ b/.claude/skills/security-tracker-stats-dashboard/SKILL.md @@ -1,16 +1,6 @@ --- name: security-tracker-stats-dashboard -description: | - Generate a self-contained HTML dashboard of `` repository - statistics: issue-lifecycle bands (untriaged / triaged / PR-merged / - fixed-released / closed-other), opened-vs-untriaged backlog, - cumulative opened/closed, mean time to triage, mean time to first - response, and — when `` is configured — mean time - createdAt -> PR-opened, PR-open -> PR-merged, and PR-merged -> - advisory announced. All charts are line / area (no bars) with - `connectgaps: true`. Vertical annotations on every chart mark the - milestones declared in the project's overlay (e.g. "skill - adoption", "team handover", "process change"). +description: Generate a self-contained HTML dashboard of `` repository statistics for security-team review. when_to_use: | Invoke when the user says "regenerate the tracker dashboard", "show monthly/quarterly stats", "tracker stats", "dashboard", or diff --git a/tools/skill-validator/src/skill_validator/__init__.py b/tools/skill-validator/src/skill_validator/__init__.py index 3b16ffe6..b26876f5 100644 --- a/tools/skill-validator/src/skill_validator/__init__.py +++ b/tools/skill-validator/src/skill_validator/__init__.py @@ -141,12 +141,14 @@ INJECTION_GUARD_TODO_CATEGORY = "injection_guard_todo" BODY_INLINE_CATEGORY = "body_inline" +GH_LIST_CATEGORY = "gh_list_no_limit" SOFT_CATEGORIES: frozenset[str] = frozenset( { PRINCIPLE_CATEGORY, TRIGGER_PRESERVATION_CATEGORY, INJECTION_GUARD_TODO_CATEGORY, BODY_INLINE_CATEGORY, + GH_LIST_CATEGORY, } ) @@ -912,6 +914,45 @@ def validate_body_inline(path: Path, text: str) -> Iterable[Violation]: ) +# --------------------------------------------------------------------------- +# gh list --limit check +# --------------------------------------------------------------------------- + +_GH_LIST_RE = re.compile(r"\bgh\s+(issue|pr)\s+list\b") + + +def _join_continuations(block_body: str) -> str: + r"""Join shell line-continuations (trailing ``\``) within a fenced block.""" + return re.sub(r"\\\n\s*", " ", block_body) + + +def validate_gh_list_limit(path: Path, text: str) -> Iterable[Violation]: + """Flag ``gh issue list`` / ``gh pr list`` in fenced blocks without ``--limit``. + + Unbounded list calls silently return GitHub CLI's default page size, so + downstream counts or filters can operate on an incomplete result set. + """ + for block_match in _FENCED_CODE_RE.finditer(text): + joined = _join_continuations(block_match.group()) + for cmd_match in _GH_LIST_RE.finditer(joined): + line_start = joined.rfind("\n", 0, cmd_match.start()) + 1 + line_end = joined.find("\n", cmd_match.end()) + if line_end == -1: + line_end = len(joined) + logical_line = joined[line_start:line_end] + if "--limit" in logical_line: + continue + line_no = text[: block_match.start()].count("\n") + joined[: cmd_match.start()].count("\n") + 1 + yield Violation( + path, + line_no, + f"gh-list-no-limit: `{cmd_match.group()}` has no `--limit` — " + f"unbounded list calls silently cap at 30 results on large repos; " + f"add `--limit ` (or `--limit 100` as a safe default)", + category=GH_LIST_CATEGORY, + ) + + def collect_doc_files(root: Path | None = None) -> set[Path]: """Return every .md file under docs/ and projects/_template/.""" repo_root = root or find_repo_root() @@ -949,6 +990,7 @@ def run_validation(root: Path | None = None) -> list[Violation]: violations.extend(validate_links(path, text, skill_dirs, doc_files)) violations.extend(validate_placeholders(path, text)) violations.extend(validate_body_inline(path, text)) + violations.extend(validate_gh_list_limit(path, text)) return violations @@ -1011,6 +1053,7 @@ def main(argv: list[str] | None = None) -> int: "parenthetical rationale", "trigger phrase", "injection-guard TODO", + "gh-list-no-limit", ) diff --git a/tools/skill-validator/tests/test_validator.py b/tools/skill-validator/tests/test_validator.py index 1a506937..f33259da 100644 --- a/tools/skill-validator/tests/test_validator.py +++ b/tools/skill-validator/tests/test_validator.py @@ -26,6 +26,7 @@ from skill_validator import ( BODY_INLINE_CATEGORY, FORBIDDEN_PATTERNS, + GH_LIST_CATEGORY, INJECTION_GUARD_CALLOUT_SENTINEL, INJECTION_GUARD_CATEGORY, INJECTION_GUARD_TODO_CATEGORY, @@ -49,6 +50,7 @@ slugify, validate_body_inline, validate_frontmatter, + validate_gh_list_limit, validate_injection_guard, validate_links, validate_placeholders, @@ -932,6 +934,62 @@ def test_soft_categories_set(self) -> None: assert TRIGGER_PRESERVATION_CATEGORY in SOFT_CATEGORIES assert INJECTION_GUARD_TODO_CATEGORY in SOFT_CATEGORIES assert BODY_INLINE_CATEGORY in SOFT_CATEGORIES + assert GH_LIST_CATEGORY in SOFT_CATEGORIES + + +# --------------------------------------------------------------------------- +# gh list --limit check +# --------------------------------------------------------------------------- + + +def _fenced(cmd: str) -> str: + """Wrap a command in a fenced bash block.""" + return f"```bash\n{cmd}\n```\n" + + +class TestGhListLimit: + def test_fires_for_gh_issue_list_no_limit(self, tmp_path: Path) -> None: + path = tmp_path / "SKILL.md" + violations = list(validate_gh_list_limit(path, _fenced("gh issue list --repo "))) + assert any("gh-list-no-limit" in v.message for v in violations) + + def test_fires_for_gh_pr_list_no_limit(self, tmp_path: Path) -> None: + path = tmp_path / "SKILL.md" + violations = list(validate_gh_list_limit(path, _fenced("gh pr list --repo "))) + assert any("gh-list-no-limit" in v.message for v in violations) + + def test_fires_on_sub_doc(self, tmp_path: Path) -> None: + path = tmp_path / "actions.md" + violations = list(validate_gh_list_limit(path, _fenced("gh pr list --repo --state open"))) + assert any("gh-list-no-limit" in v.message for v in violations) + + def test_violation_is_soft_category(self, tmp_path: Path) -> None: + path = tmp_path / "SKILL.md" + violations = list(validate_gh_list_limit(path, _fenced("gh issue list --repo "))) + assert all(v.category == GH_LIST_CATEGORY for v in violations) + + def test_silent_when_limit_on_same_line(self, tmp_path: Path) -> None: + path = tmp_path / "SKILL.md" + violations = list(validate_gh_list_limit(path, _fenced("gh issue list --repo --limit 100"))) + assert not any("gh-list-no-limit" in v.message for v in violations) + + def test_silent_when_limit_on_continuation_line(self, tmp_path: Path) -> None: + path = tmp_path / "selectors.md" + text = _fenced("gh pr list \\\n --repo \\\n --state open \\\n --limit 100") + violations = list(validate_gh_list_limit(path, text)) + assert not any("gh-list-no-limit" in v.message for v in violations) + + def test_silent_for_inline_backtick_mention(self, tmp_path: Path) -> None: + path = tmp_path / "SKILL.md" + text = "Use `gh issue list` with `--limit` to avoid truncation.\n" + violations = list(validate_gh_list_limit(path, text)) + assert not any("gh-list-no-limit" in v.message for v in violations) + + def test_silent_outside_fenced_block(self, tmp_path: Path) -> None: + path = tmp_path / "SKILL.md" + text = "Run gh issue list --repo to see open issues.\n" + violations = list(validate_gh_list_limit(path, text)) + assert not any("gh-list-no-limit" in v.message for v in violations) # ---------------------------------------------------------------------------