Skip to content

feat: add trigger mode (no permissions, server posts as bot account)#2

Open
redoh wants to merge 2 commits into
mainfrom
feat/trigger-mode
Open

feat: add trigger mode (no permissions, server posts as bot account)#2
redoh wants to merge 2 commits into
mainfrom
feat/trigger-mode

Conversation

@redoh

@redoh redoh commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

What & why

Adds a new mode: trigger input. In trigger mode the action posts no token and the workflow needs no permissions — it only notifies the Octopus server which PR to review (POST /api/oss-review with owner/repo/prNumber/headSha). The server fetches the diff and posts the review as the Octopus bot account, which (as a non-collaborator) can only comment — never set checks/statuses or block merges.

This lets security-conscious public repos adopt Octopus without granting any write access in CI and without pull_request_target. Directly addresses the maintainer concern on kubernetes/minikube#23177.

# .github/workflows/octopus.yml
name: octopus
on:
  pull_request:
    types: [opened, synchronize]
permissions: {}
jobs:
  review:
    runs-on: ubuntu-latest
    steps:
      - uses: octopusreview/action@v1
        with:
          mode: trigger

(plus a .github/octopus.yml consent file in the repo)

Changes

  • New mode input (full default = existing behavior; trigger = notify-only).
  • triggerOssReview() client call to /api/oss-review.
  • index.ts: early trigger branch; github-token only required in full mode.
  • Rebuilt dist/.

Backward compatibility

mode defaults to full; existing workflows are unchanged.

Status

Companion server PR: octopusreview/octopus#536. Not yet validated end-to-end (needs the bot account live).

🤖 Generated with Claude Code

New `mode: trigger` input. In trigger mode the action posts no token and the
workflow needs no permissions — it only notifies the Octopus server which PR to
review (POST /api/oss-review with owner/repo/prNumber/headSha). The server fetches
the diff and posts the review as the Octopus bot account, which (as a
non-collaborator) can only comment, never set checks/statuses or block merges.

This lets security-conscious public repos adopt Octopus without granting any write
access in CI and without pull_request_target. Consent is gated server-side on the
repo's .github/octopus.yml file.

Default mode is "full" — existing behavior (fetch diff + post with github-token)
is unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@octopus-review

octopus-review Bot commented Jun 19, 2026

Copy link
Copy Markdown

🐙 Octopus Review — PR #2

Summary

This PR adds a trigger mode to the GitHub Action that allows zero-permission workflows: instead of fetching the diff and posting the review itself, the action simply notifies the Octopus server which then performs the review and posts it via its own bot account. This is architecturally sound for fork PRs and security-conscious environments.

Score

Category Score Notes
Security 4/5 Unauthenticated endpoint previously flagged, now protected by consent-file check server-side
Code Quality 4/5 Clean implementation; minor unresolved logic issue with invalid mode fallback
Performance 5/5 No performance concerns
Error Handling 4/5 Errors downgraded to warnings; appropriate for async trigger pattern
Consistency 4/5 Follows existing patterns well
Overall 4/5 Solid implementation with one minor logic issue

Risk Assessment

Metric Value
Overall Risk 🟢 Low
Complexity Low
Test Coverage Impact Needs Attention
Breaking Change No

Positive Highlights

  • Clean early-exit pattern: The trigger mode block exits cleanly before any token/octokit usage, making it impossible to accidentally use a missing token downstream.
  • Graceful error handling: Trigger failures are downgraded to core.warning rather than thrown, which is the right call for an async fire-and-forget pattern — a failed trigger shouldn't block the CI pipeline.
  • Good no-consent-file UX: The distinct warning with a doc link for the opt-in file gives users clear, actionable guidance rather than a generic error.

Important Files Changed

Filename Overview
src/index.ts Adds mode input reading, validation, and the trigger-mode early-exit block
src/octopus-client.ts Adds TriggerResponse interface and triggerOssReview() function targeting /api/oss-review
action.yml Exposes the new mode input with default "full"
dist/index.js Compiled output, mirrors source changes

Diagram

sequenceDiagram
    participant WF as GitHub Actions Workflow
    participant Action as octopus-action
    participant API as Octopus Server

    WF->>Action: trigger with mode=trigger

    Action->>Action: validate event, read inputs
    Note over Action: no github-token required

    Action->>API: POST /api/oss-review (owner, repo, prNumber, headSha)
    activate API

    alt status = queued
        API-->>Action: { status: "queued", jobId }
        Action->>WF: info log + set outputs (count=0)
        API->>API: fetch diff, index, review
        API->>WF: post review as bot account
    else status = deduped
        API-->>Action: { status: "deduped" }
        Action->>WF: info log + set outputs (count=0)
    else status = skipped / no-consent-file
        API-->>Action: { status: "skipped", reason: "no-consent-file" }
        deactivate API
        Action->>WF: warning + set outputs (count=0)
    end
Loading

Last reviewed commit: 0ee1ac3

Checklist

  • No hardcoded secrets or credentials
  • Error handling is comprehensive
  • Edge cases are covered — invalid mode value still executes trigger path silently
  • Naming is clear and consistent with codebase conventions
  • No unnecessary dependencies added
  • findings-count / summary outputs are documented for trigger mode behavior in README

@octopus-review octopus-review Bot 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.

4 files reviewed, 3 findings | View scores & details

🟡🔵 Additional findings
Severity File Title Description
🟡 src/octopus-client.ts:L93 Unauthenticated trigger endpoint exposes server-side review to arbitrary repos The triggerOssReview call sends only owner, repo, prNumber, and headSha — no API key and no GitHub token. Any actor that can call the Octopus API can trigger a server-side review on any public repo by guessing these values (all of which are public). If the server performs expensive indexing and posts comments as the bot account, this is a potential resource-abuse and review-spam vector. The no-consent-file guard is a good first layer, but it relies entirely on server-side enforcement with no request authentication.
🔵 src/index.ts:L33 Invalid mode values silently fall through to full mode The mode input is read with .toLowerCase() and then compared only against "trigger". Any other value — including typos like "triggers", "Trigger" (already handled by toLowerCase), or "trgger" — silently falls through to full mode. This can mask misconfiguration.
🔵 src/index.ts:L68 Trigger mode does not set findings-count or summary outputs In trigger mode, the function returns early without calling core.setOutput. Downstream workflow steps that depend on findings-count or summary outputs (e.g., the example in README.md L148-L152) will receive empty strings, potentially causing silent failures in conditional steps.

Reviewed by Octopus Review, an AI-powered PR review tool.

- Warn when `mode` is neither "full" nor "trigger" instead of silently falling
  through to full mode (masks misconfiguration).
- Set findings-count and summary outputs in trigger mode so downstream steps that
  read them don't get empty strings. The review is posted asynchronously by the
  server, so findings-count is "0" with an explanatory summary.

The "unauthenticated trigger" finding is addressed server-side in octopusreview/octopus#536
(manual approval allowlist); a fork-PR action cannot authenticate by design.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@redoh

redoh commented Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review 🙏 Addressed in the latest push:

  • 🟡 Unauthenticated trigger (octopus-client.ts) — addressed server-side in feat(oss): server-side bot-account review mode (trigger-only, comment-only) octopus#536 via a manual approval allowlist (only approved repos are ever reviewed). A fork-PR action cannot authenticate by design, so the guard belongs on the server. 👍
  • 🔵 Invalid mode silently falls through (index.ts:33) — fixed: now emits core.warning when mode is neither full nor trigger. 👍
  • 🔵 Trigger mode doesn't set outputs (index.ts:68) — fixed: sets findings-count and summary in trigger mode so downstream steps don't get empty strings (review is posted asynchronously, so count is 0 with an explanatory summary). 👍

@octopus updates are ready for re-review.

@octopus-review octopus-review Bot 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.

4 files reviewed, 0 findings | View scores & details

Reviewed by Octopus Review, an AI-powered PR review tool.

@nirs

nirs commented Jun 21, 2026

Copy link
Copy Markdown

The diagram does not mention the case when the review is rejected early using the allow list on octopus server. The review bot does not update the diagram when the code changes or the code does not handle this case?

Stale pr message is very common issue with humans. It would be nice if the review bot always update the pr comments after changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants