forked from farhoodlabs/paperclip
b2496c8067
## Thinking Path > - Paperclip is the control plane for autonomous AI companies, so authenticated board access has to be predictable across local and worktree deployments. > - This change sits in the authenticated-mode server startup and Better Auth origin-trust wiring. > - The original auth branch fixed one real gap by adding port-qualified trusted origins for allowed hostnames on non-default ports. > - Review of that branch found a second-order bug: trusted origins were still derived from the configured port before startup detected the actual listen port. > - In isolated worktrees, that meant a common `3100 -> 3101` port shift could still leave Better Auth trusting the stale origin. > - This pull request keeps the original allowed-hostname port-variant fix, then moves trust derivation onto the resolved listen port and adds regression coverage around startup wiring. > - The benefit is that authenticated sessions keep working on allowed private hostnames even when Paperclip has to auto-shift to a different local port. ## What Changed - Added `:port` trusted-origin variants for authenticated-mode `allowedHostnames` when Paperclip runs on non-default ports. - Changed authenticated startup so `listenPort` is detected before Better Auth initialization, and explicit auth base URLs are rewritten before auth startup. - Updated `deriveAuthTrustedOrigins()` to accept the resolved listen port so Better Auth trusts the actual browser origin instead of the stale configured port. - Added focused regression coverage in `server/src/__tests__/better-auth.test.ts` and `server/src/__tests__/server-startup-feedback-export.test.ts`. ## Verification - `pnpm exec vitest run server/src/__tests__/better-auth.test.ts server/src/__tests__/server-startup-feedback-export.test.ts` - Reviewer re-check: reviewed commits `380f5b9f` and `092bb34c` after the follow-up fix landed and found no remaining issues. ## Risks - Low risk: this only affects authenticated-mode origin derivation and startup ordering around detected listen ports. - Main behavioral shift: startup no longer mutates `config.port` to the selected port; it now carries `requestedListenPort` separately and uses `listenPort` where runtime behavior needs the resolved value. - If another path was implicitly relying on `config.port` being overwritten during startup, that path would need follow-up, though the current startup/test coverage did not reveal one. > I checked `ROADMAP.md` and did not find an overlapping planned core work item for this auth trusted-origin port handling fix. ## Model Used - OpenAI Codex via Paperclip `codex_local` agents for implementation and review. Exact backend model ID/context window were not surfaced in this run context; work was performed through the Codex local adapter with tool use, code execution, and review passes. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] If this change affects the UI, I have included before/after screenshots - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] I will address all Greptile and reviewer comments before requesting merge
79 lines
2.8 KiB
TypeScript
79 lines
2.8 KiB
TypeScript
import { afterEach, describe, expect, it } from "vitest";
|
|
import type { BetterAuthOptions } from "better-auth";
|
|
import { getCookies } from "better-auth/cookies";
|
|
import {
|
|
buildBetterAuthAdvancedOptions,
|
|
deriveAuthCookiePrefix,
|
|
deriveAuthTrustedOrigins,
|
|
} from "../auth/better-auth.js";
|
|
|
|
const ORIGINAL_INSTANCE_ID = process.env.PAPERCLIP_INSTANCE_ID;
|
|
|
|
afterEach(() => {
|
|
if (ORIGINAL_INSTANCE_ID === undefined) delete process.env.PAPERCLIP_INSTANCE_ID;
|
|
else process.env.PAPERCLIP_INSTANCE_ID = ORIGINAL_INSTANCE_ID;
|
|
});
|
|
|
|
describe("Better Auth cookie scoping", () => {
|
|
it("derives an instance-scoped cookie prefix", () => {
|
|
expect(deriveAuthCookiePrefix("default")).toBe("paperclip-default");
|
|
expect(deriveAuthCookiePrefix("PAP-1601-worktree")).toBe("paperclip-PAP-1601-worktree");
|
|
});
|
|
|
|
it("uses PAPERCLIP_INSTANCE_ID for the Better Auth cookie prefix", () => {
|
|
process.env.PAPERCLIP_INSTANCE_ID = "sat-worktree";
|
|
|
|
const advanced = buildBetterAuthAdvancedOptions({ disableSecureCookies: false });
|
|
|
|
expect(advanced).toEqual({
|
|
cookiePrefix: "paperclip-sat-worktree",
|
|
});
|
|
expect(getCookies({ advanced } as BetterAuthOptions).sessionToken.name).toBe(
|
|
"paperclip-sat-worktree.session_token",
|
|
);
|
|
});
|
|
|
|
it("keeps local http auth cookies non-secure while preserving the scoped prefix", () => {
|
|
process.env.PAPERCLIP_INSTANCE_ID = "pap-worktree";
|
|
|
|
expect(buildBetterAuthAdvancedOptions({ disableSecureCookies: true })).toEqual({
|
|
cookiePrefix: "paperclip-pap-worktree",
|
|
useSecureCookies: false,
|
|
});
|
|
});
|
|
|
|
it("adds hostname port variants for authenticated mode on non-default ports", () => {
|
|
const trustedOrigins = deriveAuthTrustedOrigins({
|
|
deploymentMode: "authenticated",
|
|
authBaseUrlMode: "auto",
|
|
authPublicBaseUrl: undefined,
|
|
allowedHostnames: ["Board.Example.Test"],
|
|
port: 3101,
|
|
} as Parameters<typeof deriveAuthTrustedOrigins>[0]);
|
|
|
|
expect(trustedOrigins).toEqual(expect.arrayContaining([
|
|
"https://board.example.test",
|
|
"http://board.example.test",
|
|
"https://board.example.test:3101",
|
|
"http://board.example.test:3101",
|
|
]));
|
|
});
|
|
|
|
it("prefers an explicit resolved listen port over the configured port", () => {
|
|
const trustedOrigins = deriveAuthTrustedOrigins({
|
|
deploymentMode: "authenticated",
|
|
authBaseUrlMode: "auto",
|
|
authPublicBaseUrl: undefined,
|
|
allowedHostnames: ["board.example.test"],
|
|
port: 3100,
|
|
} as Parameters<typeof deriveAuthTrustedOrigins>[0], { listenPort: 3101 });
|
|
|
|
expect(trustedOrigins).toEqual(expect.arrayContaining([
|
|
"https://board.example.test:3101",
|
|
"http://board.example.test:3101",
|
|
]));
|
|
expect(trustedOrigins).not.toContain("https://board.example.test:3100");
|
|
expect(trustedOrigins).not.toContain("http://board.example.test:3100");
|
|
});
|
|
});
|