Skip to content

Commit 86df3db

Browse files
committed
refactor: address review comments — shared escaping utility and deduplicated handler
- Extract escapeContextKeyValue() into db/pagination.ts as shared utility for pipe-delimiter escaping in context keys (was inline in issue/list.ts and project/list.ts). - Deduplicate handleResolvedTargets overrides in issue/list.ts: single resolveAndHandle function reused across auto-detect, explicit, and project-search modes instead of three identical closures. - Add explanatory comments for fullQuery || undefined in api-client.ts and for 'Project' ContextError resource in org-list.ts handleProjectSearch.
1 parent a757e7a commit 86df3db

File tree

5 files changed

+50
-32
lines changed

5 files changed

+50
-32
lines changed

src/commands/issue/list.ts

Lines changed: 24 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,14 @@ import {
1313
listIssuesPaginated,
1414
listProjects,
1515
} from "../../lib/api-client.js";
16-
import { parseOrgProjectArg } from "../../lib/arg-parsing.js";
16+
import {
17+
type ParsedOrgProject,
18+
parseOrgProjectArg,
19+
} from "../../lib/arg-parsing.js";
1720
import { buildCommand } from "../../lib/command.js";
1821
import {
1922
clearPaginationCursor,
23+
escapeContextKeyValue,
2024
resolveOrgCursor,
2125
setPaginationCursor,
2226
} from "../../lib/db/pagination.js";
@@ -407,8 +411,9 @@ type OrgAllIssuesOptions = {
407411
async function handleOrgAllIssues(options: OrgAllIssuesOptions): Promise<void> {
408412
const { stdout, org, flags, setContext } = options;
409413
// Encode sort + query in context key so cursors from different searches don't collide.
410-
// Pipe chars in user query are escaped to prevent delimiter injection.
411-
const escapedQuery = flags.query?.replaceAll("|", "%7C");
414+
const escapedQuery = flags.query
415+
? escapeContextKeyValue(flags.query)
416+
: undefined;
412417
const contextKey = `host:${getApiBaseUrl()}|type:org:${org}|sort:${flags.sort}${escapedQuery ? `|q:${escapedQuery}` : ""}`;
413418
const cursor = resolveOrgCursor(flags.cursor, PAGINATION_KEY, contextKey);
414419

@@ -685,40 +690,29 @@ export const listCommand = buildCommand({
685690

686691
const parsed = parseOrgProjectArg(target);
687692

693+
// Shared handler for modes that resolve targets (auto-detect, explicit, project-search).
694+
// Each mode's narrowed ParsedVariant<K> is a subtype of ParsedOrgProject,
695+
// so passing it to handleResolvedTargets (which accepts the full union) is safe.
696+
const resolveAndHandle = (p: ParsedOrgProject) =>
697+
handleResolvedTargets({
698+
stdout,
699+
stderr,
700+
parsed: p,
701+
flags,
702+
cwd,
703+
setContext,
704+
});
705+
688706
await dispatchOrgScopedList({
689707
config: issueListMeta,
690708
stdout,
691709
cwd,
692710
flags,
693711
parsed,
694712
overrides: {
695-
"auto-detect": (p) =>
696-
handleResolvedTargets({
697-
stdout,
698-
stderr,
699-
parsed: p,
700-
flags,
701-
cwd,
702-
setContext,
703-
}),
704-
explicit: (p) =>
705-
handleResolvedTargets({
706-
stdout,
707-
stderr,
708-
parsed: p,
709-
flags,
710-
cwd,
711-
setContext,
712-
}),
713-
"project-search": (p) =>
714-
handleResolvedTargets({
715-
stdout,
716-
stderr,
717-
parsed: p,
718-
flags,
719-
cwd,
720-
setContext,
721-
}),
713+
"auto-detect": resolveAndHandle,
714+
explicit: resolveAndHandle,
715+
"project-search": resolveAndHandle,
722716
"org-all": (p) =>
723717
handleOrgAllIssues({ stdout, org: p.org, flags, setContext }),
724718
},

src/commands/project/list.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import { buildCommand } from "../../lib/command.js";
2727
import { getDefaultOrganization } from "../../lib/db/defaults.js";
2828
import {
2929
clearPaginationCursor,
30+
escapeContextKeyValue,
3031
resolveOrgCursor,
3132
setPaginationCursor,
3233
} from "../../lib/db/pagination.js";
@@ -216,9 +217,8 @@ export function buildContextKey(
216217
}
217218
if (flags.platform) {
218219
// Normalize to lowercase since platform filtering is case-insensitive.
219-
// Escape pipe chars to prevent delimiter injection.
220220
parts.push(
221-
`platform:${flags.platform.toLowerCase().replaceAll("|", "%7C")}`
221+
`platform:${escapeContextKeyValue(flags.platform.toLowerCase())}`
222222
);
223223
}
224224
return parts.join("|");

src/lib/api-client.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1037,6 +1037,9 @@ export function listIssuesPaginated(
10371037
`/organizations/${orgSlug}/issues/`,
10381038
{
10391039
params: {
1040+
// Convert empty string to undefined so ky omits the param entirely;
1041+
// sending `query=` causes the Sentry API to behave differently than
1042+
// omitting the parameter.
10401043
query: fullQuery || undefined,
10411044
cursor: options.cursor,
10421045
per_page: options.perPage ?? 25,
@@ -1228,6 +1231,9 @@ export async function listTransactions(
12281231
dataset: "transactions",
12291232
field: TRANSACTION_FIELDS,
12301233
project: isNumericProject ? projectSlug : undefined,
1234+
// Convert empty string to undefined so ky omits the param entirely;
1235+
// sending `query=` causes the Sentry API to behave differently than
1236+
// omitting the parameter.
12311237
query: fullQuery || undefined,
12321238
per_page: options.limit || 10,
12331239
statsPeriod: options.statsPeriod ?? "7d",

src/lib/db/pagination.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,21 @@ export function clearPaginationCursor(
103103
).run(commandKey, context);
104104
}
105105

106+
/**
107+
* Escape a user-provided value for safe inclusion in a context key.
108+
*
109+
* Context keys use `|` as a segment delimiter. If user input (e.g., a search
110+
* query or platform filter) contains `|`, it must be escaped to prevent
111+
* delimiter injection that could cause cache collisions between different
112+
* query combinations.
113+
*
114+
* @param value - Raw user-provided string
115+
* @returns Escaped string with `|` replaced by `%7C`
116+
*/
117+
export function escapeContextKeyValue(value: string): string {
118+
return value.replaceAll("|", "%7C");
119+
}
120+
106121
/**
107122
* Build a context key for org-scoped pagination cursor storage.
108123
*

src/lib/org-list.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -521,6 +521,9 @@ export async function handleProjectSearch<TEntity, TWithOrg>(
521521
writeJson(stdout, []);
522522
return;
523523
}
524+
// Use "Project" as the resource name (not config.entityName) because the
525+
// error is about a project lookup failure, not the entity being listed.
526+
// e.g., "Project is missing" not "Team is missing".
524527
throw new ContextError(
525528
"Project",
526529
`No project '${projectSlug}' found in any accessible organization.\n\n` +

0 commit comments

Comments
 (0)