diff --git a/README.md b/README.md index 8700dc3..e8c30e4 100644 --- a/README.md +++ b/README.md @@ -171,8 +171,9 @@ needed for creation or status changes. The bot polls GitHub once a day for new or changed security advisories (including Triage/draft reports) across every nf-core repo and posts them to a -private Slack channel for the core team to review — see -[docs/security-advisories.md](docs/security-advisories.md). +private Slack channel for the core team to review. `@core-team` members can also +run `/nf-core security summary` any time for an on-demand summary of all open +advisories — see [docs/security-advisories.md](docs/security-advisories.md). ## Development diff --git a/docs/commands.md b/docs/commands.md index 4ebfa4d..c36ee9a 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -495,13 +495,43 @@ when the schedule changes. --- +## Security Commands + +These commands surface GitHub security advisory data. All require `@core-team` +Slack user group membership. See +[security-advisories.md](security-advisories.md) for the daily automated digest +that runs alongside these. + +### `security help` + +``` +/nf-core security help +``` + +Show security command help. + +### `security summary` + +``` +/nf-core security summary +``` + +Fetches live data from GitHub (not the cached digest state) and posts a summary +of every open advisory (`triage`, `draft`, or `published` — excludes +`closed`/`withdrawn`) across all nf-core repos, grouped by repo. Posted +**visibly** in the conversation where you run it (channel or DM), not +ephemerally, so anyone there can see the result. Falls back to DMing you if the +bot can't post where the command was run (e.g. a channel it isn't a member of). + +--- + ## Permission Model -| Level | Who | Access | -| ------------------ | -------------------------------------------- | -------------------------------------------------------------------------------------- | -| **User** | Everyone | `/hackathon list`, `/hackathon register/edit/cancel`, `/hackathon sites` | -| **Site organiser** | Per-hackathon, per-site (stored in DynamoDB) | `/hackathon export` | -| **Admin** | `@core-team` Slack user group members | All commands including `/hackathon admin *`, `/nf-core github *`, `/nf-core on-call *` | +| Level | Who | Access | +| ------------------ | -------------------------------------------- | ------------------------------------------------------------------------------------------------------------- | +| **User** | Everyone | `/hackathon list`, `/hackathon register/edit/cancel`, `/hackathon sites` | +| **Site organiser** | Per-hackathon, per-site (stored in DynamoDB) | `/hackathon export` | +| **Admin** | `@core-team` Slack user group members | All commands including `/hackathon admin *`, `/nf-core github *`, `/nf-core on-call *`, `/nf-core security *` | Admin membership is checked via `usergroups.users.list` and cached for 5 minutes. diff --git a/docs/security-advisories.md b/docs/security-advisories.md index bee2242..f74b75c 100644 --- a/docs/security-advisories.md +++ b/docs/security-advisories.md @@ -34,6 +34,14 @@ flowchart TD Runs as a background `asyncio.Task` launched from `app.py`, following the same pattern as the on-call scheduler (`scheduler/oncall_jobs.py`). +## On-demand summary + +`/nf-core security summary` (`@core-team` only) fetches live data from GitHub +and posts a summary of every currently-open advisory across all repos, grouped +by repo — independent of the daily digest's new/changed diffing. Posted visibly +wherever the command is run (channel or DM), not ephemerally. See +[commands.md](commands.md#security-commands). + ## Configuration (env vars) | Var | Default | Notes | @@ -69,3 +77,5 @@ belongs to. state (`ADVISORY#{repo}` / `{ghsa_id}`) - `src/nf_core_bot/security/advisories.py` — the scheduler loop, diffing logic, and Slack posting (digest + failure notices) +- `src/nf_core_bot/commands/security/summary.py` — the `security summary` slash + command handler diff --git a/src/nf_core_bot/commands/help.py b/src/nf_core_bot/commands/help.py index fb5ba76..a23db54 100644 --- a/src/nf_core_bot/commands/help.py +++ b/src/nf_core_bot/commands/help.py @@ -48,6 +48,10 @@ ("on-call calendar me", "Get a subscribable iCal URL for your on-call weeks", "admin"), ] +SECURITY_COMMANDS: list[tuple[str, str, str]] = [ + ("security summary", "Summarise all open GitHub security advisories across nf-core repos", "admin"), +] + GENERAL_COMMANDS: list[tuple[str, str, str]] = [ ("help", "Show this help message", "all"), ( @@ -57,6 +61,7 @@ ), ("on-call help", "Show on-call rotation commands (@core-team only)", "admin"), ("github help", "Show GitHub commands (@core-team only)", "admin"), + ("security help", "Show security commands (@core-team only)", "admin"), ] @@ -174,3 +179,16 @@ async def handle_oncall_help( sections.append(_format_commands(ONCALL_COMMANDS)) await respond("\n".join(sections), response_type="ephemeral") + + +async def handle_security_help( + respond: Respond, +) -> None: + """Security help: ``/nf-core security help``. + + Only visible to ``@core-team`` members (permission checked in router). + """ + sections = ["*Security commands* (`@core-team` only)\n"] + sections.append(_format_commands(SECURITY_COMMANDS)) + + await respond("\n".join(sections), response_type="ephemeral") diff --git a/src/nf_core_bot/commands/router.py b/src/nf_core_bot/commands/router.py index d718fdf..39d5470 100644 --- a/src/nf_core_bot/commands/router.py +++ b/src/nf_core_bot/commands/router.py @@ -30,7 +30,13 @@ handle_edit, handle_register, ) -from nf_core_bot.commands.help import handle_github_help, handle_hackathon_help, handle_help, handle_oncall_help +from nf_core_bot.commands.help import ( + handle_github_help, + handle_hackathon_help, + handle_help, + handle_oncall_help, + handle_security_help, +) from nf_core_bot.commands.oncall.calendar_cmd import handle_oncall_calendar from nf_core_bot.commands.oncall.list_cmd import handle_oncall_list from nf_core_bot.commands.oncall.me import handle_oncall_me @@ -39,6 +45,7 @@ from nf_core_bot.commands.oncall.switch import handle_oncall_switch from nf_core_bot.commands.oncall.unavailable import handle_oncall_unavailable from nf_core_bot.commands.proposals.review import handle_proposals_review +from nf_core_bot.commands.security.summary import handle_security_summary from nf_core_bot.permissions.checks import is_core_team, is_core_team_or_maintainers if TYPE_CHECKING: @@ -108,6 +115,11 @@ async def dispatch( await _route_proposals(ack, respond, client, user_id, command, rest) return + # ── Security commands ───────────────────────────────────────────── + if sub == "security": + await _route_security(ack, respond, client, user_id, command, rest) + return + # ── Unknown ────────────────────────────────────────────────────── await ack() await respond( @@ -348,3 +360,41 @@ async def _route_proposals( f"Unknown proposals command: `{sub}`. {_PROPOSALS_HELP}", response_type="ephemeral", ) + + +# ── Security dispatch ──────────────────────────────────────────────── + + +async def _route_security( + ack: Ack, + respond: Respond, + client: AsyncWebClient, + user_id: str, + command: dict[str, str], + tokens: list[str], +) -> None: + """Dispatch ``/nf-core security [args…]``. + + Restricted to ``@core-team`` members. + """ + await ack() + + if not await is_core_team(client, user_id): + await respond( + "Sorry, security commands are restricted to `@core-team` members.", + response_type="ephemeral", + ) + return + + if not tokens or tokens[0].lower() == "help": + await handle_security_help(respond) + return + + sub = tokens[0].lower() + if sub == "summary": + await handle_security_summary(respond, client, user_id, command) + else: + await respond( + f"Unknown security command: `{sub}`. Run `/nf-core security help` for options.", + response_type="ephemeral", + ) diff --git a/src/nf_core_bot/commands/security/__init__.py b/src/nf_core_bot/commands/security/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/src/nf_core_bot/commands/security/summary.py b/src/nf_core_bot/commands/security/summary.py new file mode 100644 index 0000000..0847ccb --- /dev/null +++ b/src/nf_core_bot/commands/security/summary.py @@ -0,0 +1,67 @@ +"""``/nf-core security summary`` — summarise open GitHub security advisories. + +Restricted to ``@core-team`` (checked by the router). Posts **publicly** in +the conversation where the command was run — a channel or DM — so the whole +team can see the result, not just the caller, mirroring +``commands/proposals/review.py``'s delivery pattern. +""" + +from __future__ import annotations + +import logging +from typing import TYPE_CHECKING + +from slack_sdk.errors import SlackApiError + +from nf_core_bot.checks.github import list_all_org_advisories +from nf_core_bot.security.advisories import format_advisories_summary + +if TYPE_CHECKING: + from slack_bolt.context.respond.async_respond import AsyncRespond as Respond + from slack_sdk.web.async_client import AsyncWebClient + +logger = logging.getLogger(__name__) + +# Posting where the command was run can fail when the bot isn't a member there +# (a personal self-DM, a channel it hasn't joined) — fall back to a DM instead. +_DM_FALLBACK_ERRORS = frozenset({"channel_not_found", "not_in_channel", "is_archived"}) + + +async def handle_security_summary( + respond: Respond, + client: AsyncWebClient, + user_id: str, + command: dict[str, str], +) -> None: + """Fetch live advisory data from GitHub and post a public summary.""" + try: + advisories = await list_all_org_advisories() + except Exception: + logger.exception("Failed to fetch org security advisories for /nf-core security summary") + await respond( + "Could not fetch security advisories from GitHub — check the bot's token/permissions.", + response_type="ephemeral", + ) + return + + text = format_advisories_summary(advisories) + channel_id = command.get("channel_id", "") + + try: + await client.chat_postMessage(channel=channel_id, text=text) + except SlackApiError as e: + if e.response.get("error") not in _DM_FALLBACK_ERRORS: + raise + try: + dm = (await client.conversations_open(users=[user_id]))["channel"]["id"] + await client.chat_postMessage(channel=dm, text=text) + await respond( + ":inbox_tray: Posted the summary to our DM — I couldn't post where you ran the command.", + response_type="ephemeral", + ) + except SlackApiError: + logger.exception("Failed to deliver security summary for caller %s", user_id) + await respond( + "Sorry — I couldn't deliver the summary. Try running the command in a channel I'm a member of.", + response_type="ephemeral", + ) diff --git a/src/nf_core_bot/security/advisories.py b/src/nf_core_bot/security/advisories.py index fb72e44..91a84c8 100644 --- a/src/nf_core_bot/security/advisories.py +++ b/src/nf_core_bot/security/advisories.py @@ -105,6 +105,36 @@ async def _post_digest(client: AsyncWebClient, advisories: list[SecurityAdvisory logger.exception("Failed to post security advisory %s to Slack", adv.ghsa_id) +# Advisory states that are resolved/done, so are excluded from "open". +_CLOSED_STATES = frozenset({"closed", "withdrawn"}) + + +def format_advisories_summary(advisories: list[SecurityAdvisory]) -> str: + """Render a summary of all open advisories, grouped by repo, for ``/nf-core security summary``. + + "Open" excludes ``closed``/``withdrawn`` states — ``triage``, ``draft`` and + ``published`` all still need attention. Summaries are attacker-influenced + (see ``_escape_mrkdwn``), same as the daily digest. + """ + open_advisories = [a for a in advisories if a.state not in _CLOSED_STATES] + if not open_advisories: + return ":white_check_mark: No open security advisories across nf-core repos." + + by_repo: dict[str, list[SecurityAdvisory]] = {} + for adv in open_advisories: + by_repo.setdefault(adv.repo, []).append(adv) + + count = len(open_advisories) + lines = [f":closed_lock_with_key: *{count} open security advisor{'y' if count == 1 else 'ies'} across nf-core:*"] + for repo in sorted(by_repo): + lines.append(f"\n*{_escape_mrkdwn(repo)}*") + for adv in by_repo[repo]: + severity = f", {adv.severity}" if adv.severity else "" + summary = _escape_mrkdwn(adv.summary) + lines.append(f"• <{adv.url}|{summary}> (`{adv.state}`{severity}) — `{adv.ghsa_id}`") + return "\n".join(lines) + + async def _notify(client: AsyncWebClient, message: str) -> None: """Post a check-failure notice to the security channel — never fail silently.""" channel = await resolve_channel_id(client, config.SECURITY_CHANNEL or "security") diff --git a/tests/test_security_advisories.py b/tests/test_security_advisories.py index ad0a793..af99852 100644 --- a/tests/test_security_advisories.py +++ b/tests/test_security_advisories.py @@ -236,3 +236,64 @@ async def test_no_post_when_nothing_flagged(self, _mock_advisories, _mock_ch, dd await check_security_advisories(client) client.chat_postMessage.assert_not_awaited() + + +# ── security.advisories: format_advisories_summary ───────────────────── + + +class TestFormatAdvisoriesSummary: + def test_no_open_advisories(self) -> None: + from nf_core_bot.security.advisories import format_advisories_summary + + assert "No open security advisories" in format_advisories_summary([]) + + def test_excludes_closed_and_withdrawn(self) -> None: + from nf_core_bot.security.advisories import format_advisories_summary + + advisories = [ + SecurityAdvisory("rnaseq", "GHSA-a", "fixed one", "closed", "high", "2026-08-01", "https://x"), + SecurityAdvisory("rnaseq", "GHSA-b", "withdrawn one", "withdrawn", "high", "2026-08-01", "https://x"), + ] + summary = format_advisories_summary(advisories) + assert "No open security advisories" in summary + + def test_groups_by_repo_and_includes_open_states(self) -> None: + from nf_core_bot.security.advisories import format_advisories_summary + + advisories = [ + SecurityAdvisory("rnaseq", "GHSA-a", "triage issue", "triage", "high", "2026-08-01", "https://x/a"), + SecurityAdvisory("sarek", "GHSA-b", "published issue", "published", None, "2026-08-02", "https://x/b"), + SecurityAdvisory("rnaseq", "GHSA-c", "old closed", "closed", "low", "2026-08-01", "https://x/c"), + ] + summary = format_advisories_summary(advisories) + + assert "2 open security advisories" in summary + assert summary.index("*rnaseq*") < summary.index("*sarek*") + assert "" in summary + assert "`triage`" in summary and "high" in summary + assert "" in summary + assert "GHSA-c" not in summary + + def test_singular_count(self) -> None: + from nf_core_bot.security.advisories import format_advisories_summary + + advisories = [SecurityAdvisory("rnaseq", "GHSA-a", "leak", "triage", "high", "2026-08-01", "https://x")] + assert "1 open security advisory " in format_advisories_summary(advisories) + + def test_escapes_mrkdwn_in_summary_and_repo(self) -> None: + from nf_core_bot.security.advisories import format_advisories_summary + + advisories = [ + SecurityAdvisory( + "rnaseq", + "GHSA-a", + "leak> ", + "triage", + "high", + "2026-08-01", + "https://x", + ) + ] + summary = format_advisories_summary(advisories) + assert "" not in summary + assert "leak> <https://evil.example.com|click>" in summary diff --git a/tests/test_security_command.py b/tests/test_security_command.py new file mode 100644 index 0000000..8468423 --- /dev/null +++ b/tests/test_security_command.py @@ -0,0 +1,98 @@ +"""Tests for the /nf-core security summary slash command + router gate.""" + +from __future__ import annotations + +from unittest.mock import AsyncMock, patch + +from slack_sdk.errors import SlackApiError + +from nf_core_bot.commands import router +from nf_core_bot.commands.security import summary as cmd + + +class TestHandler: + async def test_posts_summary_where_command_was_run(self) -> None: + respond = AsyncMock() + client = AsyncMock() + with ( + patch.object(cmd, "list_all_org_advisories", new=AsyncMock(return_value=[])), + patch.object(cmd, "format_advisories_summary", return_value="SUMMARY TEXT"), + ): + await cmd.handle_security_summary(respond, client, "U1", {"channel_id": "C1"}) + + client.chat_postMessage.assert_awaited_once_with(channel="C1", text="SUMMARY TEXT") + respond.assert_not_awaited() + + async def test_fetch_failure_reports_ephemerally(self) -> None: + respond = AsyncMock() + client = AsyncMock() + with patch.object(cmd, "list_all_org_advisories", new=AsyncMock(side_effect=RuntimeError("403"))): + await cmd.handle_security_summary(respond, client, "U1", {"channel_id": "C1"}) + + client.chat_postMessage.assert_not_awaited() + respond.assert_awaited_once() + assert "Could not fetch" in respond.await_args.args[0] + assert respond.await_args.kwargs.get("response_type") == "ephemeral" + + async def test_falls_back_to_dm_when_not_in_channel(self) -> None: + respond = AsyncMock() + client = AsyncMock() + client.conversations_open = AsyncMock(return_value={"channel": {"id": "D9"}}) + err = SlackApiError("nope", {"error": "channel_not_found"}) + client.chat_postMessage = AsyncMock(side_effect=[err, {"ok": True}]) + with ( + patch.object(cmd, "list_all_org_advisories", new=AsyncMock(return_value=[])), + patch.object(cmd, "format_advisories_summary", return_value="SUMMARY TEXT"), + ): + await cmd.handle_security_summary(respond, client, "U1", {"channel_id": "C_selfdm"}) + + assert client.chat_postMessage.await_count == 2 + assert client.chat_postMessage.await_args_list[1].kwargs["channel"] == "D9" + assert respond.await_args.kwargs.get("response_type") == "ephemeral" + + async def test_reraises_unexpected_slack_errors(self) -> None: + respond = AsyncMock() + client = AsyncMock() + err = SlackApiError("nope", {"error": "some_other_error"}) + client.chat_postMessage = AsyncMock(side_effect=err) + with ( + patch.object(cmd, "list_all_org_advisories", new=AsyncMock(return_value=[])), + patch.object(cmd, "format_advisories_summary", return_value="SUMMARY TEXT"), + ): + try: + await cmd.handle_security_summary(respond, client, "U1", {"channel_id": "C1"}) + except SlackApiError: + pass + else: + raise AssertionError("expected SlackApiError to propagate") + + +class TestRouterGate: + async def test_non_member_denied(self) -> None: + ack, respond = AsyncMock(), AsyncMock() + with patch.object(router, "is_core_team", new=AsyncMock(return_value=False)): + await router._route_security(ack, respond, AsyncMock(), "U1", {"channel_id": "C1"}, ["summary"]) + ack.assert_awaited_once() + assert "restricted" in respond.await_args.args[0] + + async def test_member_dispatched(self) -> None: + ack, respond = AsyncMock(), AsyncMock() + handler = AsyncMock() + with ( + patch.object(router, "is_core_team", new=AsyncMock(return_value=True)), + patch.object(router, "handle_security_summary", handler), + ): + await router._route_security(ack, respond, AsyncMock(), "U1", {"channel_id": "C1"}, ["summary"]) + handler.assert_awaited_once() + + async def test_help(self) -> None: + ack, respond = AsyncMock(), AsyncMock() + with patch.object(router, "is_core_team", new=AsyncMock(return_value=True)): + await router._route_security(ack, respond, AsyncMock(), "U1", {"channel_id": "C1"}, []) + assert "security summary" in respond.await_args.args[0] + + async def test_unknown_subcommand(self) -> None: + ack, respond = AsyncMock(), AsyncMock() + with patch.object(router, "is_core_team", new=AsyncMock(return_value=True)): + await router._route_security(ack, respond, AsyncMock(), "U1", {"channel_id": "C1"}, ["bogus"]) + assert "Unknown security command" in respond.await_args.args[0]