Skip to content

OAuth phishing-resistance: consent screen, HMAC nonce, redirect policy, client cap #106

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
jpr5 merged 6 commits into from
Jun 15, 2026
Merged

OAuth phishing-resistance: consent screen, HMAC nonce, redirect policy, client cap #106

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6ee7402
Add OAuth observability, config plumbing, and trusted-IP resolver
jpr5 Jun 15, 2026
a94d709
Add HMAC consent-nonce and redirect_uri policy
jpr5 Jun 15, 2026
dc7fece
Add OAuth client cap with TTL eviction and touch
jpr5 Jun 15, 2026
e151de3
Add OAuth consent screen and POST consent handler
jpr5 Jun 15, 2026
8d1066d
Wire OAuth phishing-resistance into handlers and server
jpr5 Jun 15, 2026
6d720d5
Format with prettier
jpr5 Jun 15, 2026
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
6 changes: 6 additions & 0 deletions src/__tests__/analytics-endpoints.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ vi.mock("../config.js", () => ({
discordPublicKey: "",
notionToken: "",
mcpJwtSecret: "x".repeat(32),
oauthConsentHmacKeys: ["a".repeat(64)],
p2pTelemetryUrl: undefined,
p2pTelemetryDisabled: false,
packageVersion: "test",
Expand Down Expand Up @@ -83,6 +84,7 @@ const DEFAULT_TEST_CONFIG = {
discordPublicKey: "",
notionToken: "",
mcpJwtSecret: "x".repeat(32),
oauthConsentHmacKeys: ["a".repeat(64)],
p2pTelemetryUrl: undefined,
p2pTelemetryDisabled: false,
packageVersion: "test",
Expand Down Expand Up @@ -387,6 +389,7 @@ describe("analyticsAuth middleware", () => {
discordPublicKey: "",
notionToken: "",
mcpJwtSecret: "x".repeat(32),
oauthConsentHmacKeys: ["a".repeat(64)],
p2pTelemetryUrl: undefined,
p2pTelemetryDisabled: false,
packageVersion: "test",
Expand Down Expand Up @@ -428,6 +431,7 @@ describe("analyticsAuth middleware", () => {
discordPublicKey: "",
notionToken: "",
mcpJwtSecret: "x".repeat(32),
oauthConsentHmacKeys: ["a".repeat(64)],
p2pTelemetryUrl: undefined,
p2pTelemetryDisabled: false,
packageVersion: "test",
Expand Down Expand Up @@ -471,6 +475,7 @@ describe("analyticsAuth middleware", () => {
discordPublicKey: "",
notionToken: "",
mcpJwtSecret: "x".repeat(32),
oauthConsentHmacKeys: ["a".repeat(64)],
p2pTelemetryUrl: undefined,
p2pTelemetryDisabled: false,
packageVersion: "test",
Expand Down Expand Up @@ -516,6 +521,7 @@ describe("analyticsAuth middleware", () => {
discordPublicKey: "",
notionToken: "",
mcpJwtSecret: "x".repeat(32),
oauthConsentHmacKeys: ["a".repeat(64)],
p2pTelemetryUrl: undefined,
p2pTelemetryDisabled: false,
packageVersion: "test",
Expand Down
6 changes: 6 additions & 0 deletions src/__tests__/analytics-server.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ vi.mock("../config.js", () => ({
discordPublicKey: "",
notionToken: "",
mcpJwtSecret: "x".repeat(32),
oauthConsentHmacKeys: ["a".repeat(64)],
p2pTelemetryUrl: undefined,
p2pTelemetryDisabled: false,
packageVersion: "test",
Expand Down Expand Up @@ -953,6 +954,7 @@ describe("Analytics server routes (HTTP-level)", () => {
discordPublicKey: "",
notionToken: "",
mcpJwtSecret: "x".repeat(32),
oauthConsentHmacKeys: ["a".repeat(64)],
p2pTelemetryUrl: undefined,
p2pTelemetryDisabled: false,
packageVersion: "test",
Expand Down Expand Up @@ -994,6 +996,7 @@ describe("Analytics server routes (HTTP-level)", () => {
discordPublicKey: "",
notionToken: "",
mcpJwtSecret: "x".repeat(32),
oauthConsentHmacKeys: ["a".repeat(64)],
p2pTelemetryUrl: undefined,
p2pTelemetryDisabled: false,
packageVersion: "test",
Expand Down Expand Up @@ -1021,6 +1024,7 @@ describe("Analytics server routes (HTTP-level)", () => {
discordPublicKey: "",
notionToken: "",
mcpJwtSecret: "x".repeat(32),
oauthConsentHmacKeys: ["a".repeat(64)],
p2pTelemetryUrl: undefined,
p2pTelemetryDisabled: false,
packageVersion: "test",
Expand Down Expand Up @@ -1061,6 +1065,7 @@ describe("Analytics server routes (HTTP-level)", () => {
discordPublicKey: "",
notionToken: "",
mcpJwtSecret: "x".repeat(32),
oauthConsentHmacKeys: ["a".repeat(64)],
p2pTelemetryUrl: undefined,
p2pTelemetryDisabled: false,
packageVersion: "test",
Expand Down Expand Up @@ -1115,6 +1120,7 @@ describe("Analytics server routes (HTTP-level)", () => {
discordPublicKey: "",
notionToken: "",
mcpJwtSecret: "x".repeat(32),
oauthConsentHmacKeys: ["a".repeat(64)],
p2pTelemetryUrl: undefined,
p2pTelemetryDisabled: false,
packageVersion: "test",
Expand Down
2 changes: 2 additions & 0 deletions src/__tests__/atlas-ratification-endpoints.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ vi.mock("../config.js", async (importOriginal) => {
discordPublicKey: "",
notionToken: "",
mcpJwtSecret: "x".repeat(32),
oauthConsentHmacKeys: ["a".repeat(64)],
p2pTelemetryUrl: undefined,
p2pTelemetryDisabled: false,
packageVersion: "test",
Expand Down Expand Up @@ -73,6 +74,7 @@ const DEFAULT_TEST_CONFIG = {
discordPublicKey: "",
notionToken: "",
mcpJwtSecret: "x".repeat(32),
oauthConsentHmacKeys: ["a".repeat(64)],
p2pTelemetryUrl: undefined,
p2pTelemetryDisabled: false,
packageVersion: "test",
Expand Down
2 changes: 2 additions & 0 deletions src/__tests__/atlas-search-endpoint.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ vi.mock("../config.js", async (importOriginal) => {
discordPublicKey: "",
notionToken: "",
mcpJwtSecret: "x".repeat(32),
oauthConsentHmacKeys: ["a".repeat(64)],
p2pTelemetryUrl: undefined,
p2pTelemetryDisabled: false,
packageVersion: "test",
Expand Down Expand Up @@ -104,6 +105,7 @@ const DEFAULT_TEST_CONFIG = {
discordPublicKey: "",
notionToken: "",
mcpJwtSecret: "x".repeat(32),
oauthConsentHmacKeys: ["a".repeat(64)],
p2pTelemetryUrl: undefined,
p2pTelemetryDisabled: false,
packageVersion: "test",
Expand Down
128 changes: 128 additions & 0 deletions src/__tests__/config.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1136,6 +1136,7 @@ describe("config.ts", () => {
process.env.GITHUB_TOKEN = "ghp_test";
process.env.GITHUB_WEBHOOK_SECRET = "secret";
process.env.MCP_JWT_SECRET = "x".repeat(64);
process.env.PATHFINDER_CONSENT_HMAC_KEY = "a".repeat(64);

mockedExistsSync.mockReturnValue(true);
mockedReadFileSync.mockReturnValue(makeYaml());
Expand Down Expand Up @@ -1465,6 +1466,133 @@ describe("config.ts", () => {
});
});

// ── oauthConsentHmacKeys ─────────────────────────────────────────────────

describe("oauthConsentHmacKeys", () => {
beforeEach(() => {
process.env.PATHFINDER_CONFIG = "/tmp/test.yaml";
process.env.DATABASE_URL = "postgresql://test";
process.env.OPENAI_API_KEY = "sk-test";
});

it("parses a single PATHFINDER_CONSENT_HMAC_KEY", async () => {
const k = "a".repeat(64);
process.env.PATHFINDER_CONSENT_HMAC_KEY = k;
mockedExistsSync.mockReturnValue(true);
mockedReadFileSync.mockReturnValue(makeYaml());

const { getConfig } = await freshImport();
const cfg = getConfig();
expect(cfg.oauthConsentHmacKeys).toEqual([k]);
});

it("parses comma-separated keys for rotation", async () => {
const k1 = "a".repeat(64);
const k2 = "b".repeat(48);
process.env.PATHFINDER_CONSENT_HMAC_KEY = `${k1},${k2}`;
mockedExistsSync.mockReturnValue(true);
mockedReadFileSync.mockReturnValue(makeYaml());

const { getConfig } = await freshImport();
const cfg = getConfig();
expect(cfg.oauthConsentHmacKeys).toEqual([k1, k2]);
});

it("trims whitespace around comma-separated entries", async () => {
const k1 = "a".repeat(64);
const k2 = "b".repeat(64);
process.env.PATHFINDER_CONSENT_HMAC_KEY = ` ${k1} , ${k2} `;
mockedExistsSync.mockReturnValue(true);
mockedReadFileSync.mockReturnValue(makeYaml());

const { getConfig } = await freshImport();
const cfg = getConfig();
expect(cfg.oauthConsentHmacKeys).toEqual([k1, k2]);
});

it("rejects keys shorter than 32 hex chars", async () => {
process.env.PATHFINDER_CONSENT_HMAC_KEY = "abcdef"; // way too short
mockedExistsSync.mockReturnValue(true);
mockedReadFileSync.mockReturnValue(makeYaml());

const { getConfig } = await freshImport();
expect(() => getConfig()).toThrow(
/PATHFINDER_CONSENT_HMAC_KEY entries must be ≥32 hex chars/,
);
});

it("rejects non-hex characters in keys", async () => {
// 64 chars but contains 'g' (not hex)
process.env.PATHFINDER_CONSENT_HMAC_KEY = "g".repeat(64);
mockedExistsSync.mockReturnValue(true);
mockedReadFileSync.mockReturnValue(makeYaml());

const { getConfig } = await freshImport();
expect(() => getConfig()).toThrow(
/PATHFINDER_CONSENT_HMAC_KEY entries must be ≥32 hex chars/,
);
});

it("rejects when any key in a rotation list is invalid", async () => {
const good = "a".repeat(64);
const bad = "short";
process.env.PATHFINDER_CONSENT_HMAC_KEY = `${good},${bad}`;
mockedExistsSync.mockReturnValue(true);
mockedReadFileSync.mockReturnValue(makeYaml());

const { getConfig } = await freshImport();
expect(() => getConfig()).toThrow(
/PATHFINDER_CONSENT_HMAC_KEY entries must be ≥32 hex chars/,
);
});

it("throws in production when PATHFINDER_CONSENT_HMAC_KEY is unset", async () => {
process.env.NODE_ENV = "production";
process.env.MCP_JWT_SECRET = "x".repeat(64);
delete process.env.PATHFINDER_CONSENT_HMAC_KEY;
mockedExistsSync.mockReturnValue(true);
mockedReadFileSync.mockReturnValue(makeYaml());

const { getConfig } = await freshImport();
expect(() => getConfig()).toThrow(
/PATHFINDER_CONSENT_HMAC_KEY is required in production/,
);
});

it("generates an ephemeral key in development with a WARN log", async () => {
process.env.NODE_ENV = "development";
delete process.env.PATHFINDER_CONSENT_HMAC_KEY;
mockedExistsSync.mockReturnValue(true);
mockedReadFileSync.mockReturnValue(makeYaml());
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});

const { getConfig } = await freshImport();
const cfg = getConfig();
expect(cfg.oauthConsentHmacKeys).toHaveLength(1);
expect(cfg.oauthConsentHmacKeys[0]).toMatch(/^[0-9a-f]{64}$/);
expect(warnSpy).toHaveBeenCalledWith(
expect.stringMatching(
/\[oauth\] PATHFINDER_CONSENT_HMAC_KEY not set — generated an ephemeral consent-nonce key for development\./,
),
);
warnSpy.mockRestore();
});

it("treats whitespace-only PATHFINDER_CONSENT_HMAC_KEY as unset (dev fallback)", async () => {
process.env.NODE_ENV = "development";
process.env.PATHFINDER_CONSENT_HMAC_KEY = " ";
mockedExistsSync.mockReturnValue(true);
mockedReadFileSync.mockReturnValue(makeYaml());
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});

const { getConfig } = await freshImport();
const cfg = getConfig();
expect(cfg.oauthConsentHmacKeys).toHaveLength(1);
expect(cfg.oauthConsentHmacKeys[0]).toMatch(/^[0-9a-f]{64}$/);
warnSpy.mockRestore();
});
});

// ── Config caching ───────────────────────────────────────────────────────

describe("caching", () => {
Expand Down
Loading