Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ All notable changes to this project will be documented in this file.

## [Unreleased]

### Fixed
- **Self-hosted: only auto-respond to PR change-requests on PRs Cyrus owns** — `pull_request_review` events with state `changes_requested` now trigger Cyrus only when the PR was opened by the configured Cyrus bot account or its description contains the hidden Cyrus marker. Other PRs are ignored (with a debug log). Explicit `@cyrusagent` mentions on any PR continue to work. ([CYPACK-1173](https://linear.app/ceedar/issue/CYPACK-1173), [#1186](https://github.com/cyrusagents/cyrus/pull/1186))

## [0.2.57] - 2026-05-22

### Fixed
Expand Down
15 changes: 15 additions & 0 deletions packages/edge-worker/src/EdgeWorker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ import {
isIssueCommentPayload,
isPullRequestReviewCommentPayload,
isPullRequestReviewPayload,
PullRequestReviewAuthorizer,
stripMention,
} from "cyrus-github-event-transport";
import type { GitLabWebhookEvent } from "cyrus-gitlab-event-transport";
Expand Down Expand Up @@ -1194,6 +1195,20 @@ export class EdgeWorker extends EventEmitter {
);
return;
}

// Only auto-respond to change requests on PRs Cyrus owns —
// either authored by the Cyrus bot or carrying the hidden
// Cyrus marker in the description. Explicit @mentions are
// handled separately and remain allowed on any PR.
const authorization = new PullRequestReviewAuthorizer({
botUsername,
}).authorize(event.payload);
if (!authorization.authorized) {
this.logger.debug(
`Ignoring pull_request_review on ${repoFullName}#${prNumber}: ${authorization.reason}`,
);
return;
}
}

// Only trigger on comments that mention the bot (when configured)
Expand Down
8 changes: 5 additions & 3 deletions packages/edge-worker/src/hooks/PrMarkerHook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@ import type {
PostToolUseHookInput,
} from "cyrus-claude-runner";
import type { ILogger } from "cyrus-core";
import { CYRUS_PR_MARKER } from "cyrus-github-event-transport";

/**
* The hidden HTML marker that identifies a PR/MR description as Cyrus-authored.
* Its presence is what tells our GitHub/GitLab webhook handlers that a
* "Changes requested" or comment event should be forwarded back to Cyrus.
* Re-exported from cyrus-github-event-transport so that the marker-injection
* hook (here) and the PR review authorization policy (there) share a single
* source of truth.
*/
export const CYRUS_PR_MARKER = "<!-- generated-by-cyrus -->";
export { CYRUS_PR_MARKER };

/**
* Provider-specific knowledge about how to detect PR/MR mutating commands and
Expand Down
97 changes: 97 additions & 0 deletions packages/github-event-transport/src/PullRequestReviewAuthorizer.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
import type { GitHubPullRequestReviewPayload, GitHubUser } from "./types.js";

/**
* Hidden HTML marker that identifies a PR description as Cyrus-authored.
*
* Canonical location for the marker — both the marker-injection hook (in
* cyrus-edge-worker) and the review authorization policy (here) read from
* this constant so the two sides cannot drift.
*/
export const CYRUS_PR_MARKER = "<!-- generated-by-cyrus -->";

/**
* Configuration for {@link PullRequestReviewAuthorizer}.
*/
export interface PullRequestReviewAuthorizerConfig {
/**
* Configured Cyrus GitHub bot login (e.g. `cyrusagent` or
* `cyrusagent[bot]`). Used to recognise PRs opened by Cyrus.
*
* When undefined, only the hidden-marker check is performed.
*/
botUsername?: string;
}

/**
* Outcome of a PR-review authorization check.
*/
export interface PullRequestReviewAuthorization {
authorized: boolean;
/** Human-readable reason, suitable for debug logging. */
reason: string;
}

/**
* Decides whether a `pull_request_review` event from GitHub is allowed to
* trigger Cyrus.
*
* Mirrors the policy enforced by the hosted webhook handler in
* `cyrus-hosted/apps/app/src/app/api/github/webhook/route.ts`: a review
* counts as actionable only when either
* 1. the PR author is the Cyrus bot account, or
* 2. the PR body contains {@link CYRUS_PR_MARKER}.
*
* Loop prevention (ignoring reviews authored *by* the bot) is the caller's
* responsibility — this policy is purely about PR ownership.
*/
export class PullRequestReviewAuthorizer {
constructor(
private readonly config: PullRequestReviewAuthorizerConfig = {},
) {}

authorize(
payload: GitHubPullRequestReviewPayload,
): PullRequestReviewAuthorization {
const prUser = payload.pull_request.user;
const prBody = payload.pull_request.body ?? "";

if (this.isCyrusBotAuthor(prUser)) {
return {
authorized: true,
reason: `PR author @${prUser.login} matches the configured Cyrus bot account`,
};
}

if (prBody.includes(CYRUS_PR_MARKER)) {
return {
authorized: true,
reason: "PR body contains the hidden Cyrus marker",
};
}

return {
authorized: false,
reason: `PR author @${prUser.login} is not the Cyrus bot and PR body lacks the Cyrus marker`,
};
}

/**
* True when the PR's author looks like the configured Cyrus bot account.
*
* GitHub Apps surface as a user whose `type === "Bot"` and whose login is
* the App slug with a `[bot]` suffix (e.g. `cyrusagent[bot]`). Self-hosted
* deployments using a PAT under a regular user account surface as
* `type === "User"` with a plain login. We accept either shape, comparing
* logins case-insensitively against the configured bot username (with or
* without the `[bot]` suffix).
*/
private isCyrusBotAuthor(prUser: GitHubUser): boolean {
const { botUsername } = this.config;
if (!botUsername) return false;

const normalize = (login: string): string =>
login.toLowerCase().replace(/\[bot\]$/, "");

return normalize(prUser.login) === normalize(botUsername);
}
}
8 changes: 8 additions & 0 deletions packages/github-event-transport/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,14 @@ export {
isPullRequestReviewPayload,
stripMention,
} from "./github-webhook-utils.js";
export type {
PullRequestReviewAuthorization,
PullRequestReviewAuthorizerConfig,
} from "./PullRequestReviewAuthorizer.js";
export {
CYRUS_PR_MARKER,
PullRequestReviewAuthorizer,
} from "./PullRequestReviewAuthorizer.js";
export type {
GitHubComment,
GitHubCommentEventType,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
import { describe, expect, it } from "vitest";
import {
CYRUS_PR_MARKER,
PullRequestReviewAuthorizer,
} from "../src/PullRequestReviewAuthorizer.js";
import type { GitHubPullRequestReviewPayload } from "../src/types.js";
import { prReviewPayload, testPullRequest, testUser } from "./fixtures.js";

function payloadWith(
overrides: Partial<{
body: string | null;
userLogin: string;
userType: string;
}>,
): GitHubPullRequestReviewPayload {
return {
...prReviewPayload,
pull_request: {
...testPullRequest,
body:
"body" in overrides ? (overrides.body ?? null) : testPullRequest.body,
user: {
...testUser,
login: overrides.userLogin ?? testUser.login,
type: overrides.userType ?? testUser.type,
},
},
};
}

describe("PullRequestReviewAuthorizer", () => {
it("authorizes when the PR author matches the configured bot username", () => {
const authorizer = new PullRequestReviewAuthorizer({
botUsername: "cyrusagent",
});

const result = authorizer.authorize(
payloadWith({ userLogin: "cyrusagent", userType: "User", body: "" }),
);

expect(result.authorized).toBe(true);
expect(result.reason).toContain("Cyrus bot");
});

it("authorizes when PR author is the GitHub App bot variant (login[bot])", () => {
const authorizer = new PullRequestReviewAuthorizer({
botUsername: "cyrusagent",
});

const result = authorizer.authorize(
payloadWith({
userLogin: "cyrusagent[bot]",
userType: "Bot",
body: "",
}),
);

expect(result.authorized).toBe(true);
});

it("authorizes a PR by a human author when the body contains the marker", () => {
const authorizer = new PullRequestReviewAuthorizer({
botUsername: "cyrusagent",
});

const result = authorizer.authorize(
payloadWith({
userLogin: "alice",
userType: "User",
body: `Some description\n\n${CYRUS_PR_MARKER}\n`,
}),
);

expect(result.authorized).toBe(true);
expect(result.reason).toContain("marker");
});

it("rejects a PR by a human author with no marker", () => {
const authorizer = new PullRequestReviewAuthorizer({
botUsername: "cyrusagent",
});

const result = authorizer.authorize(
payloadWith({
userLogin: "alice",
userType: "User",
body: "Plain human PR",
}),
);

expect(result.authorized).toBe(false);
expect(result.reason).toContain("not the Cyrus bot");
});

it("rejects a PR by a human author when the PR body is null", () => {
const authorizer = new PullRequestReviewAuthorizer({
botUsername: "cyrusagent",
});

const result = authorizer.authorize(
payloadWith({
userLogin: "alice",
userType: "User",
body: null,
}),
);

expect(result.authorized).toBe(false);
});

it("falls back to marker-only when no botUsername is configured", () => {
const authorizer = new PullRequestReviewAuthorizer();

const withMarker = authorizer.authorize(
payloadWith({
userLogin: "anyone",
userType: "User",
body: CYRUS_PR_MARKER,
}),
);
const withoutMarker = authorizer.authorize(
payloadWith({
userLogin: "anyone",
userType: "User",
body: "no marker here",
}),
);

expect(withMarker.authorized).toBe(true);
expect(withoutMarker.authorized).toBe(false);
});

it("matches bot username case-insensitively", () => {
const authorizer = new PullRequestReviewAuthorizer({
botUsername: "CyrusAgent",
});

const result = authorizer.authorize(
payloadWith({
userLogin: "cyrusagent[bot]",
userType: "Bot",
body: "",
}),
);

expect(result.authorized).toBe(true);
});
});
Loading