Skip to content
Merged
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
5 changes: 4 additions & 1 deletion src/auth/github-oauth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -241,9 +241,12 @@ function normalizeReturnTo(env: Env, value: string | undefined): string {
if (!value) return fallback;
try {
const url = new URL(value, siteOrigin);
// siteOrigin already IS "https://gittensory.aethereal.dev" when PUBLIC_SITE_ORIGIN is unset (the fallback
// two lines up), so a separate hardcoded entry here was dead weight once a self-hoster sets their own
// PUBLIC_SITE_ORIGIN -- it kept accepting the cloud origin as a valid redirect target even for a self-host
// instance that never uses it (#4615). Rely solely on siteOrigin.
const allowedOrigins = new Set([
siteOrigin.replace(/\/$/, ""),
"https://gittensory.aethereal.dev",
"http://localhost:3000",
"http://localhost:4173",
"http://localhost:5173",
Expand Down
4 changes: 4 additions & 0 deletions src/env.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,10 @@ declare global {
SCORING_TIME_DECAY_ENABLED?: string;
/** #776 agent-layer GLOBAL kill-switch — when truthy, halts ALL agent actions across every repo. */
AGENT_ACTIONS_PAUSED?: string;
/** #4615: comma-separated logins ADDED to the built-in auto-close protection allowlist
* (github-actions[bot]/dependabot[bot]/renovate[bot] — settings/agent-actions.ts), for a self-hoster
* running a different automation stack (e.g. mergify[bot], snyk-bot). Additive only. */
PROTECTED_AUTOCLOSE_AUTHORS_EXTRA?: string;
/** Self-host instance-wide write switch: "dry-run" | "disabled" forces EVERY installation write to be
* suppressed regardless of per-repo mode (the cloud→self-host parallel-run kill switch). Unset = live. */
SELFHOST_DEPLOYMENT_MODE?: string;
Expand Down
24 changes: 17 additions & 7 deletions src/github/backfill.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3799,7 +3799,7 @@ async function isAuthorizedReviewThreadAuthor(
login: string | null | undefined,
association: string | null | undefined,
): Promise<boolean> {
if (isOwnReviewThreadAuthor(login)) return false;
if (isOwnReviewThreadAuthor(env, login)) return false;
if (isTrustedScannerReviewThreadAuthor(env, login)) return true;
if (isMaintainerReviewThreadAuthor(association)) return true;
return isVerifiedMemberReviewThreadAuthor(env, repoFullName, token, memberPermissionCache, admissionKey, login, association);
Expand Down Expand Up @@ -3862,12 +3862,22 @@ function isTrustedScannerReviewThreadAuthor(env: Env, login: string | null | und
.includes(normalized);
}

// Match only OUR OWN app bot login (a `gittensory` / `gittensory-orb[bot]` PREFIX), never a third-party slug
// that merely ENDS in `-gittensory[bot]`. Anchored to `^`: a `\b` boundary also fires after a hyphen, so the
// prior `\bgittensory…` misclassified e.g. `evil-gittensory[bot]` as our own author and dropped its
// review-thread comment as a self-authored non-blocker (fail-open) instead of evaluating it as external.
export function isOwnReviewThreadAuthor(login: string | null | undefined): boolean {
return /^gittensory[-\w]*\[bot\]$/i.test(login ?? "") || /^(gittensory|gittensory-orb)$/i.test(login ?? "");
// Match only OUR OWN app bot login (a `${GITHUB_APP_SLUG}` / `${GITHUB_APP_SLUG}-orb[bot]` PREFIX), never a
// third-party slug that merely ENDS in `-${GITHUB_APP_SLUG}[bot]`. Anchored to `^`: a `\b` boundary also fires
// after a hyphen, so the prior `\bgittensory…` misclassified e.g. `evil-gittensory[bot]` as our own author and
// dropped its review-thread comment as a self-authored non-blocker (fail-open) instead of evaluating it as
// external. Derived from `env.GITHUB_APP_SLUG` (#4615) rather than a hardcoded "gittensory" literal -- every
// other "is this our own bot" check in the codebase already does this (self-authored.ts, pr-actions.ts,
// comments.ts, processors.ts) -- so a self-hoster who renamed their App still recognizes its own comments.
export function isOwnReviewThreadAuthor(env: Env, login: string | null | undefined): boolean {
const slug = env.GITHUB_APP_SLUG.trim().toLowerCase();
if (!slug) return false;
const escapedSlug = escapeRegExpForOwnAuthorSlug(slug);
return new RegExp(`^${escapedSlug}[-\\w]*\\[bot\\]$`, "i").test(login ?? "") || new RegExp(`^(${escapedSlug}|${escapedSlug}-orb)$`, "i").test(login ?? "");
}

function escapeRegExpForOwnAuthorSlug(value: string): string {
return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
}

/** The deterministic linked-issue facts the hard-rule evaluator needs (labels / assignees / open-state), plus
Expand Down
29 changes: 18 additions & 11 deletions src/orb/oauth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
// surface).
import type { Context } from "hono";
import { timeoutFetch } from "../github/client";
import { GITTENSORY_SITE_URL } from "../github/footer";
import { isOrbBrokerEnabled, issueOrbEnrollment } from "./broker";

type GitHubUser = { login: string; id?: number };
Expand Down Expand Up @@ -68,20 +69,20 @@ export async function verifyInstallationAdmin(

async function handleOrbEnrollment(c: Context<{ Bindings: Env }>, code: string, installationId: number): Promise<Response> {
const token = await exchangeOrbOAuthCode(c.env, code);
if (!token) return c.html(landingPage("Couldn't verify your GitHub identity", "The authorization didn't complete — re-run the install from GitHub and try again."), 400);
if (!token) return c.html(landingPage(c.env, "Couldn't verify your GitHub identity", "The authorization didn't complete — re-run the install from GitHub and try again."), 400);
const user = await fetchOrbOAuthUser(token);
if (!user) return c.html(landingPage("Couldn't verify your GitHub identity", "We couldn't read your GitHub account — try the install again."), 400);
if (!user) return c.html(landingPage(c.env, "Couldn't verify your GitHub identity", "We couldn't read your GitHub account — try the install again."), 400);
const install = await c.env.DB.prepare("SELECT account_login, account_type, account_id, registered, self_enrollment_disabled, suspended_at, removed_at FROM orb_github_installations WHERE installation_id = ?")
.bind(installationId)
.first<{ account_login: string | null; account_type: string | null; account_id: number | null; registered: number; self_enrollment_disabled: number; suspended_at: string | null; removed_at: string | null }>();
if (!install) return c.html(landingPage("Installation not recognized", "We haven't recorded this installation yet — give it a moment after installing, then retry."), 404);
if (!install) return c.html(landingPage(c.env, "Installation not recognized", "We haven't recorded this installation yet — give it a moment after installing, then retry."), 404);
// The admin-of-installation check is the authorization gate — it runs BEFORE we reveal or change any state, so a
// non-admin learns nothing about the install and can never enroll someone else's. It binds to the immutable
// GitHub account id (logins can be renamed/reused), so a stale account_login can never grant access.
const isAdmin = await verifyInstallationAdmin(token, user.login, user.id, install.account_login, install.account_type, install.account_id);
if (!isAdmin) return c.html(landingPage("Admin access required", "You must be an admin of this installation's account to enroll it for self-host."), 403);
if (install.removed_at !== null || install.suspended_at !== null) return c.html(landingPage("Installation not active", "This installation is suspended or uninstalled — re-install the Orb App, then retry."), 403);
if (install.self_enrollment_disabled === 1) return c.html(landingPage("Installation disabled", "This installation was disabled by the operator — contact the operator to re-enable self-host enrollment."), 403);
if (!isAdmin) return c.html(landingPage(c.env, "Admin access required", "You must be an admin of this installation's account to enroll it for self-host."), 403);
if (install.removed_at !== null || install.suspended_at !== null) return c.html(landingPage(c.env, "Installation not active", "This installation is suspended or uninstalled — re-install the Orb App, then retry."), 403);
if (install.self_enrollment_disabled === 1) return c.html(landingPage(c.env, "Installation disabled", "This installation was disabled by the operator — contact the operator to re-enable self-host enrollment."), 403);
// Zero-touch self-service: a verified admin of an ACTIVE, non-disabled install self-registers it (registered=1).
// installation_id stays bound server-side in the enrollment, so brokered tokens remain scoped to this install.
if (install.registered !== 1) {
Expand All @@ -90,7 +91,7 @@ async function handleOrbEnrollment(c: Context<{ Bindings: Env }>, code: string,
const result = await issueOrbEnrollment(c.env, installationId, { login: user.login, githubId: user.id ?? null });
/* v8 ignore next -- defensive: the existence + admin + active checks above passed and we just set registered=1, so
issueOrbEnrollment (which re-checks existence + registered) cannot return an error here; kept to degrade safely. */
if ("error" in result) return c.html(landingPage("Couldn't issue an enrollment", "Please retry, or contact the operator."), 409);
if ("error" in result) return c.html(landingPage(c.env, "Couldn't issue an enrollment", "Please retry, or contact the operator."), 409);
return c.html(secretPage(result.secret));
}

Expand All @@ -104,17 +105,23 @@ export async function handleOrbOAuthCallback(c: Context<{ Bindings: Env }>): Pro
const updated = c.req.query("setup_action") === "update";
return c.html(
updated
? landingPage("Gittensory Orb updated", "Your repository selection was updated — the dashboard reflects the change shortly.")
: landingPage("Gittensory Orb connected", "Your repositories are linked. Their review activity now flows to the global Gittensory dashboard."),
? landingPage(c.env, "Gittensory Orb updated", "Your repository selection was updated — the dashboard reflects the change shortly.")
: landingPage(c.env, "Gittensory Orb connected", "Your repositories are linked. Their review activity now flows to the global Gittensory dashboard."),
);
}

function shell(heading: string, inner: string): string {
return `<!doctype html><html lang="en"><head><meta charset="utf-8"/><meta name="viewport" content="width=device-width,initial-scale=1"/><title>${heading}</title><style>*{box-sizing:border-box}body{margin:0;min-height:100vh;display:flex;align-items:center;justify-content:center;background:#0b0b0d;color:#e7e7ea;font-family:-apple-system,BlinkMacSystemFont,"Segoe UI",Roboto,sans-serif}.card{max-width:34rem;margin:1.5rem;padding:2.75rem;background:#16161a;border:1px solid #2a2a30;border-radius:14px;text-align:center}h1{font-size:1.35rem;font-weight:600;margin:0 0 .7rem}p{font-size:.95rem;line-height:1.6;color:#a8a8b0;margin:0 0 1.6rem}a{display:inline-block;padding:.6rem 1.4rem;background:#1f6feb;color:#fff;text-decoration:none;border-radius:8px;font-size:.9rem}code{background:#0b0b0d;border:1px solid #2a2a30;border-radius:6px;padding:.15rem .4rem;font-size:.85rem}pre{background:#0b0b0d;border:1px solid #2a2a30;border-radius:8px;padding:1rem;overflow:auto;text-align:left;color:#7ee787;font-size:.9rem;user-select:all}</style></head><body><div class="card"><h1>${heading}</h1>${inner}</div></body></html>`;
}

function landingPage(heading: string, message: string): string {
return shell(heading, `<p>${message}</p><a href="https://gittensory.aethereal.dev">Open the dashboard</a>`);
// The Orb App itself stays a single centrally-hosted hub (broker.ts: gittensory holds the App key centrally
// and mints tokens on demand) -- that part is architecturally fixed. This link is just where the browser lands
// after OAuth, so it follows the SAME self-hoster-configurable pattern as maintainerControlPanelUrl one
// file-family over (github/footer.ts): env.PUBLIC_SITE_ORIGIN when set, else the public gittensory dashboard
// (#4615).
function landingPage(env: Env, heading: string, message: string): string {
const dashboardOrigin = (env.PUBLIC_SITE_ORIGIN ?? GITTENSORY_SITE_URL).replace(/\/$/, "");
return shell(heading, `<p>${message}</p><a href="${dashboardOrigin}">Open the dashboard</a>`);
}

/** Show the freshly-issued enrollment secret ONCE. The secret is a generated opaque token (no user input), safe
Expand Down
16 changes: 14 additions & 2 deletions src/settings/agent-actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,20 @@ export const AGENT_LABEL_MIGRATION_COLLISION = "migration-collision";
// or slop heuristic — the maintainer owns its lifecycle. (reviewbot wrongly auto-closed such an accumulator,
// awesome-claude #4192.) Still eligible for auto-merge when clean + passing.
const PROTECTED_AUTOCLOSE_AUTHORS = new Set(["github-actions[bot]", "dependabot[bot]", "renovate[bot]"]);
export function isProtectedAutomationAuthor(login: string | null | undefined): boolean {
return login != null && PROTECTED_AUTOCLOSE_AUTHORS.has(login.toLowerCase());

// A self-hoster running a different automation stack (mergify[bot], snyk-bot, allcontributors[bot], ...) can
// extend the base allowlist with a comma-separated PROTECTED_AUTOCLOSE_AUTHORS_EXTRA env var instead of
// forking the code (#4615). Additive only -- an unset/blank value never shrinks the base set below.
function protectedAutocloseAuthors(env: Env | undefined): Set<string> {
const extra = env?.PROTECTED_AUTOCLOSE_AUTHORS_EXTRA
?.split(",")
.map((login) => login.trim().toLowerCase())
.filter(Boolean);
return extra && extra.length > 0 ? new Set([...PROTECTED_AUTOCLOSE_AUTHORS, ...extra]) : PROTECTED_AUTOCLOSE_AUTHORS;
}

export function isProtectedAutomationAuthor(login: string | null | undefined, env?: Env): boolean {
return login != null && protectedAutocloseAuthors(env).has(login.toLowerCase());
}

export type PlannedAgentAction = {
Expand Down
18 changes: 18 additions & 0 deletions test/integration/orb-oauth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,24 @@ describe("GET /v1/orb/oauth/callback (post-install landing)", () => {
const res = await app.request("/v1/orb/ingest", { method: "POST" }, createTestEnv());
expect([400, 413]).toContain(res.status); // reached the (exempt) ingest handler, failed only on the empty body
});

it("the dashboard link follows a self-hoster's PUBLIC_SITE_ORIGIN, not a hardcoded gittensory.aethereal.dev (#4615)", async () => {
const res = await app.request(
"/v1/orb/oauth/callback",
{},
createTestEnv({ PUBLIC_SITE_ORIGIN: "https://my-instance.example.com/" }),
);
const html = await res.text();
expect(html).toContain('href="https://my-instance.example.com"'); // trailing slash stripped, matching footer.ts's siteOrigin normalization
expect(html).not.toContain("gittensory.aethereal.dev");
});

it("falls back to the public gittensory dashboard when PUBLIC_SITE_ORIGIN is unset (cloud default)", async () => {
const env = createTestEnv();
delete (env as Partial<Env>).PUBLIC_SITE_ORIGIN;
const res = await app.request("/v1/orb/oauth/callback", {}, env);
expect(await res.text()).toContain('href="https://gittensory.aethereal.dev"');
});
});

describe("verifyInstallationAdmin (the privilege-escalation gate)", () => {
Expand Down
21 changes: 21 additions & 0 deletions test/unit/agent-actions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import type { GateCheckConclusion } from "../../src/rules/advisory";
// actually manifest in this test file's own module graph, not just incidentally in other suites. Importing
// agent-actions.ts alone (above) never exercises the OTHER direction of the cycle -- this import does.
import { DEFAULT_ISSUE_DISCOVERY_SHARE } from "../../src/scoring/model";
import { createTestEnv } from "../helpers/d1";

function input(overrides: Partial<AgentActionPlanInput> & { conclusion: GateCheckConclusion }): AgentActionPlanInput {
return {
Expand Down Expand Up @@ -1454,6 +1455,26 @@ describe("isProtectedAutomationAuthor", () => {
expect(isProtectedAutomationAuthor(null)).toBe(false);
expect(isProtectedAutomationAuthor(undefined)).toBe(false);
});

it("extends the base allowlist via PROTECTED_AUTOCLOSE_AUTHORS_EXTRA (#4615), additively", () => {
const env = createTestEnv({ PROTECTED_AUTOCLOSE_AUTHORS_EXTRA: "mergify[bot], Snyk-Bot" });
expect(isProtectedAutomationAuthor("mergify[bot]", env)).toBe(true);
expect(isProtectedAutomationAuthor("snyk-bot", env)).toBe(true); // case-insensitive
expect(isProtectedAutomationAuthor("github-actions[bot]", env)).toBe(true); // base set still honored
expect(isProtectedAutomationAuthor("some-other-bot[bot]", env)).toBe(false);
});

it("ignores a blank/whitespace-only PROTECTED_AUTOCLOSE_AUTHORS_EXTRA (never shrinks the base set)", () => {
const blank = createTestEnv({ PROTECTED_AUTOCLOSE_AUTHORS_EXTRA: " , ," });
expect(isProtectedAutomationAuthor("github-actions[bot]", blank)).toBe(true);
expect(isProtectedAutomationAuthor("mergify[bot]", blank)).toBe(false);
});

it("behaves exactly as before when no env (or no override) is passed", () => {
expect(isProtectedAutomationAuthor("github-actions[bot]", createTestEnv())).toBe(true);
expect(isProtectedAutomationAuthor("mergify[bot]", createTestEnv())).toBe(false);
expect(isProtectedAutomationAuthor("github-actions[bot]", undefined)).toBe(true);
});
});

describe("downgradeMergeToHold — accuracy circuit-breaker (#self-improve / GAP-4)", () => {
Expand Down
21 changes: 21 additions & 0 deletions test/unit/auth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -689,6 +689,27 @@ describe("private-beta auth and rate limiting", () => {
await expect(createSessionFromGitHubToken(env, "github-token")).resolves.toMatchObject({ login: "jsonbored", scopes: [] });
});

it("a self-hoster's PUBLIC_SITE_ORIGIN is the sole allowed returnTo origin -- the cloud origin is no longer a bonus entry (#4615)", async () => {
const env = createTestEnv({
GITHUB_OAUTH_CLIENT_ID: "client-id",
GITHUB_OAUTH_CLIENT_SECRET: "client-secret",
PUBLIC_SITE_ORIGIN: "https://my-instance.example.com",
});

// The self-hoster's own origin is accepted.
const own = await startGitHubWebOAuth(env, "https://my-instance.example.com/v1/auth/github/start", "https://my-instance.example.com/app/workbench");
expect(own.returnTo).toBe("https://my-instance.example.com/app/workbench");

// The cloud gittensory.aethereal.dev origin is no longer accepted for THIS instance -- it falls back to
// this instance's own /app, proving the dead literal is gone rather than merely redundant.
const cloud = await startGitHubWebOAuth(env, "https://my-instance.example.com/v1/auth/github/start", "https://gittensory.aethereal.dev/app/workbench");
expect(cloud.returnTo).toBe("https://my-instance.example.com/app");

// Localhost dev origins remain accepted regardless of PUBLIC_SITE_ORIGIN.
const local = await startGitHubWebOAuth(env, "https://my-instance.example.com/v1/auth/github/start", "http://localhost:5173/app");
expect(local.returnTo).toBe("http://localhost:5173/app");
});

it("verifies the GitHub token audience before minting a session on the token-exchange path", async () => {
const env = createTestEnv({ GITHUB_OAUTH_CLIENT_ID: "client-id", GITHUB_OAUTH_CLIENT_SECRET: "client-secret" });
const introspect = (appClientId: string | undefined) =>
Expand Down
Loading