Skip to content

fix(session): decode canonical detail route identities #1388

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
ding113 merged 1 commit into from
Aug 3, 2026
Merged

fix(session): decode canonical detail route identities #1388

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
26 changes: 24 additions & 2 deletions ...hboard/sessions/[sessionId]/messages/_components/session-messages-client-actions.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,10 @@ vi.mock("next-intl", () => {
});

let seqParamValue: string | null = null;
let sessionIdParamValue = "0123456789abcdef";
vi.mock("next/navigation", () => {
return {
useParams: () => ({ sessionId: "0123456789abcdef" }),
useParams: () => ({ sessionId: sessionIdParamValue }),
useSearchParams: () => ({
get: (key: string) => {
if (key !== "seq") return null;
Expand All @@ -57,7 +58,7 @@ vi.mock("@/i18n/routing", () => {

const getSessionDetailsMock = vi.fn();
const terminateActiveSessionMock = vi.fn();
vi.mock("@/actions/active-sessions", () => {
vi.mock("@/lib/api-client/v1/actions/active-sessions", () => {
return {
getSessionDetails: (...args: unknown[]) => getSessionDetailsMock(...args),
terminateActiveSession: (...args: unknown[]) => terminateActiveSessionMock(...args),
Expand Down Expand Up @@ -267,9 +268,30 @@ afterEach(() => {
routerBackMock.mockReset();
vi.useRealTimers();
seqParamValue = null;
sessionIdParamValue = "0123456789abcdef";
});

describe("SessionMessagesClient (request export actions)", () => {
test("decodes an URL-encoded canonical Session ID before loading details", async () => {
sessionIdParamValue = "pfx%3A9d403aeabe1f236d%3A1ee9a5d1bd4d98ce4bed39daca4b943e";
getSessionDetailsMock.mockResolvedValue({
ok: true,
data: buildDetailsData(),
});

const { unmount } = renderClient(<SessionMessagesClient />);
await flushEffects();

expect(getSessionDetailsMock).toHaveBeenCalledWith(
"pfx:9d403aeabe1f236d:1ee9a5d1bd4d98ce4bed39daca4b943e",
undefined,
undefined,
undefined
);

unmount();
});

test("selected seq in URL overrides currentSequence for request export", async () => {
seqParamValue = "3";
getSessionDetailsMock.mockResolvedValue({
Expand Down
13 changes: 11 additions & 2 deletions .../[locale]/dashboard/sessions/[sessionId]/messages/_components/session-messages-client.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,24 @@ import { SessionMessagesDetailsTabs } from "./session-details-tabs";
import { hasSnapshotData } from "./session-messages-guards";
import { SessionStats } from "./session-stats";

function normalizeCanonicalSessionRouteParam(sessionId: string): string {
try {
const decoded = decodeURIComponent(sessionId);
return decoded.startsWith("pfx:") || decoded.startsWith("sid:") ? decoded : sessionId;
Comment on lines +58 to +59

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve percent escapes in physical session IDs

When a valid client-supplied physical ID contains percent text that decodes to a reserved prefix, such as pfx%3Afoo, buildPublicSessionIdentity deliberately preserves it because the raw ID does not begin with pfx: or sid:. Next.js already percent-decodes dynamic route parameters (also documented in src/app/api/ip-geo/[ip]/route.ts:36), so a correctly encoded link for that ID supplies pfx%3Afoo here; this second decode changes it to pfx:foo, causing detail, request-list, export, and termination operations to target a different identity. Preserve the framework-decoded parameter rather than decoding it again.

Useful? React with 👍 / 👎.

@github-actions github-actions Bot Aug 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] [LOGIC-BUG] Double-decoding the route param can rewrite a valid physical Session ID

Why this is a problem: useParams() already exposes the decoded dynamic segment here; the repo relies on the same rule in src/app/api/ip-geo/[ip]/route.ts:36 ("Next.js already percent-decodes route params, so no manual decode."). Decoding a second time changes a raw physical ID like pfx%3Afoo into pfx:foo, which violates the identity contract in src/lib/request-identity.ts:45 where ordinary Session IDs are preserved unless they already start with pfx: or sid:. After that rewrite, detail loading, request lookup, exports, and termination all target the wrong identity.

Suggested fix:

const params = useParams<{ sessionId: string }>();
const sessionId = params.sessionId;

Then update the regression test to mock the runtime value that Next actually provides ("pfx:...") and add a case proving a physical ID such as "pfx%3Afoo" stays unchanged.

} catch {
return sessionId;
}
}

export function SessionMessagesClient() {
const t = useTranslations("dashboard.sessions");
const tErrors = useTranslations("errors");

const params = useParams();
const params = useParams<{ sessionId: string }>();
const searchParams = useSearchParams();
const router = useRouter();
const pathname = usePathname();
const sessionId = params.sessionId as string;
const sessionId = normalizeCanonicalSessionRouteParam(params.sessionId);

// URL state
const seqParam = searchParams.get("seq");
Expand Down
Loading