Skip to content

ci: pass TRIAGE_WEBHOOK_URL to octo-issue-notify#81

Merged
lml2468 merged 1 commit into
mainfrom
ci/wire-triage-webhook
Jun 8, 2026
Merged

ci: pass TRIAGE_WEBHOOK_URL to octo-issue-notify#81
lml2468 merged 1 commit into
mainfrom
ci/wire-triage-webhook

Conversation

@lml2468

@lml2468 lml2468 commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

The issue notification caller only forwarded OCTO_BOT_TOKEN, so the triage webhook mode was silently skipped and the triage agent was never invoked. Forward the org-level TRIAGE_WEBHOOK_URL secret so triage runs on new issues.

The issue notification caller only forwarded OCTO_BOT_TOKEN, so the triage
webhook mode of octo-issue-notify@v1 never received its URL and was silently
skipped β€” the issue triage agent was never invoked. Forward TRIAGE_WEBHOOK_URL
(org-level secret) so the triage agent runs on new issues.
@lml2468 lml2468 requested a review from a team as a code owner June 8, 2026 13:16
@github-actions github-actions Bot added the size/XS PR size: XS label Jun 8, 2026

@Jerry-Xin Jerry-Xin left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR is relevant to octo-admin and correctly forwards the optional triage webhook secret to the issue notification reusable workflow.

πŸ’¬ Non-blocking

  • πŸ”΅ Suggestion: .github/workflows/octo-issue-feed.yml:11 uses Mininglamp-OSS/.github/.github/workflows/octo-issue-notify.yml@v1, a mutable tag, while .github/workflows/octo-issue-feed.yml:19-21 forwards secrets. This matches the existing project pattern, so it is not blocking, but pinning secret-bearing reusable workflows to immutable SHAs would reduce supply-chain risk.

βœ… Highlights

  • .github/workflows/octo-issue-feed.yml:20-21 now passes both OCTO_BOT_TOKEN and TRIAGE_WEBHOOK_URL, aligning the caller with the reusable workflow’s optional triage webhook mode.
  • The change is narrowly scoped and does not introduce new permissions or expose the secret in logs.

@yujiawei yujiawei left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review β€” PR #81 (octo-admin)

Summary

This one-line change forwards the TRIAGE_WEBHOOK_URL secret from octo-issue-feed.yml into the reusable octo-issue-notify.yml@v1 workflow, so that the triage webhook mode is actually enabled. Previously only OCTO_BOT_TOKEN was forwarded, so the reusable workflow's optional TRIAGE_WEBHOOK_URL secret was always empty and the triage agent was never invoked. The fix is correct and minimal.

1. Verification

  • βœ… Secret name matches the reusable workflow contract. The reusable workflow Mininglamp-OSS/.github/.github/workflows/octo-issue-notify.yml@v1 declares secrets.TRIAGE_WEBHOOK_URL (required: false). The caller passes TRIAGE_WEBHOOK_URL: ${{ secrets.TRIAGE_WEBHOOK_URL }} β€” exact name match, no typo. (octo-issue-feed.yml:21)
  • βœ… Downstream actually consumes the secret. The reusable workflow forwards it to the octo-notify@v1 composite action as webhook-url: ${{ secrets.TRIAGE_WEBHOOK_URL }}, which enables "webhook mode" and POSTs the caller-built JSON payload. Cross-repo dependency chain verified end-to-end (caller β†’ reusable workflow β†’ composite action).
  • βœ… Graceful no-op when the secret is absent. TRIAGE_WEBHOOK_URL is declared required: false. If the secret is unset, secrets.TRIAGE_WEBHOOK_URL resolves to an empty string; the composite action's webhook branch is gated on if webhook_url: (after .strip()), so an empty value simply skips webhook mode without failing the job. No build/run break introduced.
  • βœ… Consistent with existing convention. This mirrors the pre-existing OCTO_BOT_TOKEN line directly above it; same @v1 ref, same secrets: block style.

2. Security review (PR classified security-sensitive)

  • βœ… No secret leakage to untrusted users. The workflow triggers on issues: [opened, reopened]. The webhook URL is a capability secret consumed only via env: inside the composite action and sent through urllib β€” it is never echoed to logs (only the static label webhook is printed), and GitHub masks registered secret values. The issue author/title never receive the secret.
  • βœ… No injection surface. The webhook payload is constructed with json.dumps over env:-provided values, and inputs are sanitized (control characters stripped, length-capped) in the reusable workflow before serialization. No ${{ }} interpolation into shell or JSON strings.
  • βœ… Minimal egress by design. The forwarded payload contains only repo/number/title/url/author/action β€” the issue body is deliberately not egressed (the triage agent fetches details itself via the GitHub API). This is appropriate for a public repo.
  • βœ… Least privilege preserved. permissions: {} is retained; no new permissions are granted by this change.

3. Items worth a human verifying (no code change required)

These are operational/config checks that cannot be confirmed from the diff alone:

  • Does the TRIAGE_WEBHOOK_URL secret actually exist at the org or repo level with the correct value? If it is unset or wrong, this change is a safe no-op but the stated goal (triage runs on new issues) will silently not happen. Recommend confirming the secret is configured and pointing at the intended triage endpoint, then opening a test issue to confirm the triage agent fires.
  • Capability-secret correctness (the URL path itself being the auth token) cannot be validated from code β€” confirm the endpoint is the intended triage receiver.

4. Non-blocking notes

  • (nit, pre-existing) Both reusable-workflow refs use the mutable tag @v1 rather than a pinned commit SHA. For supply-chain hardening, pinning org reusable workflows to a SHA is best practice. This is consistent with the existing OCTO_BOT_TOKEN line and not introduced by this PR, so it is out of scope here β€” just flagging for a future hardening pass.
  • (observation) The reusable workflow now centrally forwards only opened (not reopened), while this caller still subscribes to [opened, reopened]. Reopened events will trigger the job and then be skipped early inside the reusable workflow. This is intended per the reusable workflow's documented event policy; harmless, minor wasted run on reopen.

Verdict

The change is correct, minimal, and safe. The cross-repo secret contract is satisfied and the security posture is sound. No P0/P1 issues. The only follow-ups are operational config verifications (secret existence/value), not code defects β€” so they do not block merge.

APPROVED.

@lml2468 lml2468 merged commit 0aca760 into main Jun 8, 2026
18 of 19 checks passed
@lml2468 lml2468 deleted the ci/wire-triage-webhook branch June 8, 2026 13:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants