Skip to content

fix(orb-broker): validate App credentials only after enrollment is proven valid #2733

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
JSONbored merged 1 commit into from
Jul 3, 2026
Merged

fix(orb-broker): validate App credentials only after enrollment is proven valid #2733

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 8 additions & 6 deletions src/orb/broker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,6 @@ export type BrokerResult = { token: string; installationId: number; expiresAt: s
* install. installation_id is read from the enrollment row, never the caller; the install must still be
* registered=1 and neither suspended nor removed at mint time (the gate is re-checked, not trusted from issue). */
export async function brokerOrbToken(env: Env, secret: string): Promise<BrokerResult> {
// Validate Orb App credentials up front so a misconfiguration returns a structured error (503) rather than an
// unhandled exception that manifests as a generic 500 to the self-hosted engine.
if (!env.ORB_GITHUB_APP_ID || !env.ORB_GITHUB_APP_PRIVATE_KEY) {
console.error(JSON.stringify({ level: "error", event: "orb_broker_misconfigured", message: "ORB_GITHUB_APP_ID or ORB_GITHUB_APP_PRIVATE_KEY is not set; broker cannot mint tokens." }));
return { error: "broker_misconfigured" };
}
// Warn when TOKEN_ENCRYPTION_SECRET is absent — without it, the broker cache is bypassed and every exchange hits
// GitHub's token endpoint, dramatically increasing exposure to throttle-induced failures.
if (!env.TOKEN_ENCRYPTION_SECRET) {
Expand All @@ -84,6 +78,14 @@ export async function brokerOrbToken(env: Env, secret: string): Promise<BrokerRe
await touchLastToken(env, row.enroll_id);
return { token: cached.token, installationId: row.installation_id, expiresAt: cached.expiresAt };
}
// Validate Orb App credentials only now that the caller is proven to hold a valid, eligible enrollment — NOT
// up front. Checking credentials before the enrollment lookup would let an unauthenticated caller (any bad
// secret) distinguish "broker misconfigured" from "invalid secret" via the response code alone, leaking the
// server's deployment-config state to callers who never proved they hold a real enrollment (#2710).
if (!env.ORB_GITHUB_APP_ID || !env.ORB_GITHUB_APP_PRIVATE_KEY) {
console.error(JSON.stringify({ level: "error", event: "orb_broker_misconfigured", message: "ORB_GITHUB_APP_ID or ORB_GITHUB_APP_PRIVATE_KEY is not set; broker cannot mint tokens." }));
return { error: "broker_misconfigured" };
}
const minted = await createOrbInstallationToken(env, row.installation_id);
await cacheOrbToken(env, row.enroll_id, minted);
await touchLastToken(env, row.enroll_id);
Expand Down
50 changes: 50 additions & 0 deletions test/integration/orb-broker.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,14 @@ const seedInstall = (e: Env, id: number, cols: Record<string, string | number |
};
const brokerEnv = async (over: Partial<Env> = {}): Promise<Env> =>
createTestEnv({ ORB_BROKER_ENABLED: "true", ORB_GITHUB_APP_ID: "4139483", ORB_GITHUB_APP_PRIVATE_KEY: await pkcs8Pem(), INTERNAL_JOB_TOKEN: "dev-internal-token", ...over });
// `exactOptionalPropertyTypes` forbids `KEY: undefined` overrides, so a truly-ABSENT App credential means simply
// never setting the key (not spreading `undefined` over it) — hence a dedicated builder rather than `brokerEnv({...})`.
const brokerEnvMissingAppCreds = async (missing: "id" | "key" | "both", over: Partial<Env> = {}): Promise<Env> => {
const base: Partial<Env> = { ORB_BROKER_ENABLED: "true", INTERNAL_JOB_TOKEN: "dev-internal-token", ...over };
if (missing !== "id") base.ORB_GITHUB_APP_ID = "4139483";
if (missing !== "key") base.ORB_GITHUB_APP_PRIVATE_KEY = await pkcs8Pem();
return createTestEnv(base);
};
const tokenFetch = (token = "ghs_broker", expires = "2026-06-25T08:00:00Z") => vi.stubGlobal("fetch", async () => Response.json({ token, expires_at: expires }));
const countingTokenFetch = (expires = "2026-06-25T08:00:00Z") => {
let calls = 0;
Expand Down Expand Up @@ -154,6 +162,48 @@ describe("brokerOrbToken", () => {
await db(e).prepare("UPDATE orb_github_installations SET suspended_at=CURRENT_TIMESTAMP WHERE installation_id=302").run();
expect(await brokerOrbToken(e, secret)).toEqual({ error: "installation_not_eligible" });
});

it("#2710 regression: an invalid secret NEVER reveals broker misconfiguration — invalid_enrollment wins even with no App credentials at all", async () => {
// Before the fix, the ORB_GITHUB_APP_ID/PRIVATE_KEY check ran BEFORE the enrollment lookup, so ANY caller
// (no valid secret required) could learn the server's deployment-config state via the response code. The
// credential check must only ever run once the caller is already proven to hold a valid, eligible enrollment.
const e = await brokerEnvMissingAppCreds("both");
expect(await brokerOrbToken(e, "orbsec_totally_bogus")).toEqual({ error: "invalid_enrollment" });
});

it("returns broker_misconfigured only once the enrollment is valid+eligible and a real mint is required", async () => {
const missingAppId = await brokerEnvMissingAppCreds("id");
await seedInstall(missingAppId, 310, { registered: 1 });
const { secret: secretA } = (await issueOrbEnrollment(missingAppId, 310)) as { secret: string };
expect(await brokerOrbToken(missingAppId, secretA)).toEqual({ error: "broker_misconfigured" });

const missingPrivateKey = await brokerEnvMissingAppCreds("key");
await seedInstall(missingPrivateKey, 311, { registered: 1 });
const { secret: secretB } = (await issueOrbEnrollment(missingPrivateKey, 311)) as { secret: string };
expect(await brokerOrbToken(missingPrivateKey, secretB)).toEqual({ error: "broker_misconfigured" });
});

it("still eligible-checks BEFORE reporting broker_misconfigured (an ineligible install never leaks config state either)", async () => {
const e = await brokerEnvMissingAppCreds("both");
await seedInstall(e, 312, { registered: 1 });
const { secret } = (await issueOrbEnrollment(e, 312)) as { secret: string };
await db(e).prepare("UPDATE orb_github_installations SET removed_at=CURRENT_TIMESTAMP WHERE installation_id=312").run();
expect(await brokerOrbToken(e, secret)).toEqual({ error: "installation_not_eligible" });
});

it("serves a still-fresh cached token even with no Orb App credentials at all (mint is never reached)", async () => {
vi.useFakeTimers();
vi.setSystemTime(new Date("2026-06-25T07:00:00Z"));
const e = await brokerEnv({ TOKEN_ENCRYPTION_SECRET: "orb-cache-test-secret" });
await seedInstall(e, 313, { registered: 1 });
const { secret } = (await issueOrbEnrollment(e, 313)) as { secret: string };
tokenFetch("ghs_minted", "2026-06-25T08:00:00Z");
expect(await brokerOrbToken(e, secret)).toEqual({ token: "ghs_minted", installationId: 313, expiresAt: "2026-06-25T08:00:00Z" });

// Drop the App credentials AFTER the initial mint+cache — a still-fresh cache hit must not need them.
const eNoCreds = await brokerEnvMissingAppCreds("both", { TOKEN_ENCRYPTION_SECRET: "orb-cache-test-secret", DB: e.DB });
expect(await brokerOrbToken(eNoCreds, secret)).toEqual({ token: "ghs_minted", installationId: 313, expiresAt: "2026-06-25T08:00:00Z" });
});
});

describe("broker endpoints", () => {
Expand Down
Loading