Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 6 additions & 6 deletions src/commands/dashboard/restore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,15 @@ import {
} from "./resolve.js";

type RestoreFlags = {
readonly revision: number;
readonly revision: string;
readonly json: boolean;
readonly fields?: string[];
};

type RestoreResult = {
dashboard: DashboardDetail;
orgSlug: string;
revisionId: number;
revisionId: string;
};

function formatRestoreHuman(result: RestoreResult): string {
Expand Down Expand Up @@ -80,14 +80,14 @@ export const restoreCommand = buildCommand({
revision: {
kind: "parsed",
parse: (value: string) => {
const num = Number.parseInt(value, 10);
if (Number.isNaN(num) || num < 1) {
const revision = value.trim();
if (!revision) {
throw new ValidationError(
"--revision must be a positive integer.",
"--revision must be a non-empty revision ID.",
"revision"
);
}
return num;
return revision;
},
brief: "Revision ID to restore",
},
Expand Down
15 changes: 9 additions & 6 deletions src/commands/dashboard/revisions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,19 +64,22 @@ function formatRevisionsHuman(result: RevisionsResult): string {

type RevisionRow = {
id: string;
version: string;
title: string;
author: string;
created: string;
};

const rows: RevisionRow[] = result.revisions.map((r) => ({
id: String(r.id),
version: String(r.version),
id: r.id,
title: escapeMarkdownCell(r.title),
author: r.createdBy?.name ?? r.createdBy?.email ?? r.createdBy?.id ?? "—",
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.

Bug: The author field is not escaped with escapeMarkdownCell, which can break markdown table rendering if a user's name or email contains special characters like |.
Severity: LOW

Suggested Fix

Wrap the author value assignment with the escapeMarkdownCell() helper function to ensure any special characters are properly escaped before being rendered in the table.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: src/commands/dashboard/revisions.ts#L75

Potential issue: The `author` field, which is populated using `r.createdBy?.name`,
`r.createdBy?.email`, or `r.createdBy?.id`, is not being escaped before it is rendered
in a markdown table. If any of these values contain special markdown characters like a
pipe (`|`), it will be interpreted as a table column separator. This will break the
table's structure and corrupt the command's output for dashboards where an author's
details contain such characters. Other parts of the codebase, like
`src/commands/team/list.ts`, correctly use the `escapeMarkdownCell` helper for similar
user-provided data.

Did we get this right? 👍 / 👎 to inform future reviews.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

good catch — wrapped the author fallback chain with escapeMarkdownCell() in 56d4f35.

created: `${escapeMarkdownCell(formatRelativeTime(r.dateCreated))}\n${colorTag("muted", r.dateCreated)}`,
}));

const columns: Column<RevisionRow>[] = [
{ header: "ID", value: (r) => r.id },
{ header: "VERSION", value: (r) => r.version },
{ header: "TITLE", value: (r) => r.title },
{ header: "AUTHOR", value: (r) => r.author },
{ header: "CREATED", value: (r) => r.created },
];

Expand Down Expand Up @@ -112,7 +115,7 @@ export const revisionsCommand = buildCommand({
brief: "List dashboard revisions",
fullDescription:
"List revision history for a Sentry dashboard.\n\n" +
"Shows all saved revisions with their version numbers and timestamps.\n" +
"Shows saved revisions with their IDs, titles, authors, and timestamps.\n" +
"Use `sentry dashboard restore` to revert to a previous revision.\n\n" +
"Examples:\n" +
" sentry dashboard revisions 12345\n" +
Expand Down Expand Up @@ -231,7 +234,7 @@ export const revisionsCommand = buildCommand({
return {
hint:
`Showing ${trimmed.length} revision(s) for dashboard ${dashboardId}.${navStr}\n` +
`Restore: sentry dashboard restore ${orgSlug}/ ${dashboardId} <revision-id>\n` +
`Restore: sentry dashboard restore ${orgSlug}/ ${dashboardId} --revision <revision-id>\n` +
`Dashboard: ${url}`,
};
},
Expand Down
5 changes: 3 additions & 2 deletions src/lib/api/dashboards.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,12 +221,13 @@ export async function listDashboardRevisionsPaginated(
export async function restoreDashboardRevision(
orgSlug: string,
dashboardId: string,
revisionId: number
revisionId: string
): Promise<DashboardDetail> {
const regionUrl = await resolveOrgRegion(orgSlug);
const encodedRevisionId = encodeURIComponent(revisionId);
const { data } = await apiRequestToRegion<DashboardDetail>(
regionUrl,
`/organizations/${orgSlug}/dashboards/${dashboardId}/revisions/${revisionId}/`,
`/organizations/${orgSlug}/dashboards/${dashboardId}/revisions/${encodedRevisionId}/`,
{ method: "POST", schema: DashboardDetailSchema }
);
return data;
Expand Down
18 changes: 15 additions & 3 deletions src/types/dashboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1036,13 +1036,25 @@ export const TABLE_DISPLAY_TYPES = new Set(["table", "top_n"]);
// Dashboard revision types
// ---------------------------------------------------------------------------

/** Schema for the createdBy field in a dashboard revision */
const DashboardRevisionCreatedBySchema = z
.object({
id: z.string().optional(),
name: z.string().nullish(),
email: z.string().nullish(),
avatarType: z.string().nullish(),
avatarUrl: z.string().nullish(),
})
.passthrough();

/** Schema for a dashboard revision (from GET /dashboards/{id}/revisions/) */
export const DashboardRevisionSchema = z
.object({
id: z.number(),
version: z.number(),
id: z.string(),
title: z.string(),
dateCreated: z.string(),
dashboardId: z.number(),
createdBy: DashboardRevisionCreatedBySchema.nullable(),
source: z.string(),
})
.passthrough();

Expand Down
36 changes: 22 additions & 14 deletions test/commands/dashboard/restore.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,15 @@ function createMockContext(cwd = "/tmp") {
}

type RestoreFlags = {
readonly revision: number;
readonly revision: string;
readonly json?: boolean;
readonly fields?: string[];
};

function defaultFlags(overrides: Partial<RestoreFlags> = {}): RestoreFlags {
return {
json: false,
revision: 1,
revision: "1",
...overrides,
};
}
Expand Down Expand Up @@ -110,26 +110,30 @@ describe("dashboard restore command", () => {
test("restores dashboard and outputs JSON", async () => {
const { context, stdoutWrite } = createMockContext();
const func = await restoreCommand.loader();
await func.call(context, defaultFlags({ json: true, revision: 42 }), "123");
await func.call(
context,
defaultFlags({ json: true, revision: "42" }),
"123"
);

expect(restoreDashboardRevisionSpy).toHaveBeenCalledWith(
"test-org",
"123",
42
"42"
);

const output = stdoutWrite.mock.calls.map((c) => c[0]).join("");
const parsed = JSON.parse(output);
expect(parsed.dashboard.id).toBe("123");
expect(parsed.dashboard.title).toBe("My Dashboard");
expect(parsed.revisionId).toBe(42);
expect(parsed.revisionId).toBe("42");
expect(parsed.orgSlug).toBe("test-org");
});

test("restores dashboard and outputs human-readable format", async () => {
const { context, stdoutWrite } = createMockContext();
const func = await restoreCommand.loader();
await func.call(context, defaultFlags({ revision: 42 }), "123");
await func.call(context, defaultFlags({ revision: "42" }), "123");

const output = stdoutWrite.mock.calls.map((c) => c[0]).join("");
expect(output).toContain("Restored dashboard");
Expand Down Expand Up @@ -157,13 +161,17 @@ describe("dashboard restore command", () => {
test("uses dashboard ID from positional argument", async () => {
const { context } = createMockContext();
const func = await restoreCommand.loader();
await func.call(context, defaultFlags({ json: true, revision: 5 }), "456");
await func.call(
context,
defaultFlags({ json: true, revision: "5" }),
"456"
);

expect(resolveDashboardIdSpy).toHaveBeenCalledWith("test-org", "456");
expect(restoreDashboardRevisionSpy).toHaveBeenCalledWith(
"test-org",
"123",
5
"5"
);
});

Expand All @@ -172,7 +180,7 @@ describe("dashboard restore command", () => {
const func = await restoreCommand.loader();
await func.call(
context,
defaultFlags({ json: true, revision: 10 }),
defaultFlags({ json: true, revision: "10" }),
"my-org/",
"789"
);
Expand All @@ -185,7 +193,7 @@ describe("dashboard restore command", () => {
const func = await restoreCommand.loader();
await func.call(
context,
defaultFlags({ json: true, revision: 3 }),
defaultFlags({ json: true, revision: "3" }),
"My Dashboard Title"
);

Expand All @@ -208,12 +216,12 @@ describe("dashboard restore command", () => {

// We can't easily test the flag parsing directly, but we can verify
// the API is called with the correct revision when valid
await func.call(context, defaultFlags({ revision: 1 }), "123");
await func.call(context, defaultFlags({ revision: "1" }), "123");

expect(restoreDashboardRevisionSpy).toHaveBeenCalledWith(
"test-org",
"123",
1
"1"
);
});

Expand All @@ -225,7 +233,7 @@ describe("dashboard restore command", () => {
const func = await restoreCommand.loader();

await expect(
func.call(context, defaultFlags({ revision: 999 }), "123")
func.call(context, defaultFlags({ revision: "999" }), "123")
).rejects.toThrow();
});

Expand All @@ -236,7 +244,7 @@ describe("dashboard restore command", () => {
test("shows progress message during restore", async () => {
const { context } = createMockContext();
const func = await restoreCommand.loader();
await func.call(context, defaultFlags({ revision: 42 }), "123");
await func.call(context, defaultFlags({ revision: "42" }), "123");

expect(withProgressSpy).toHaveBeenCalled();
const [opts] = withProgressSpy.mock.calls[0] as [
Expand Down
42 changes: 29 additions & 13 deletions test/commands/dashboard/revisions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,24 +55,39 @@ function defaultFlags(overrides: Partial<RevisionsFlags> = {}): RevisionsFlags {
// ---------------------------------------------------------------------------

const REVISION_A: DashboardRevision = {
id: 1,
version: 1,
id: "1",
title: "My Dashboard",
dateCreated: "2026-01-15T10:00:00Z",
dashboardId: 123,
createdBy: {
id: "u1",
name: "Alice",
email: "alice@example.com",
avatarType: "letter_avatar",
avatarUrl: null,
},
source: "ui",
};

const REVISION_B: DashboardRevision = {
id: 2,
version: 2,
id: "2",
title: "My Dashboard (updated)",
dateCreated: "2026-02-20T12:00:00Z",
dashboardId: 123,
createdBy: {
id: "u2",
name: "Bob",
email: "bob@example.com",
avatarType: "letter_avatar",
avatarUrl: null,
},
source: "ui",
};

const REVISION_C: DashboardRevision = {
id: 3,
version: 3,
id: "3",
title: "My Dashboard (v3)",
dateCreated: "2026-03-01T08:00:00Z",
dashboardId: 123,
createdBy: null,
source: "api",
};

// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -149,9 +164,9 @@ describe("dashboard revisions command", () => {
expect(parsed).toHaveProperty("hasMore", false);
expect(parsed).toHaveProperty("hasPrev", false);
expect(parsed.data).toHaveLength(2);
expect(parsed.data[0].id).toBe(1);
expect(parsed.data[0].version).toBe(1);
expect(parsed.data[1].id).toBe(2);
expect(parsed.data[0].id).toBe("1");
expect(parsed.data[0].title).toBe("My Dashboard");
expect(parsed.data[1].id).toBe("2");
});

test("outputs { data: [], hasMore: false } when no revisions exist", async () => {
Expand Down Expand Up @@ -185,7 +200,8 @@ describe("dashboard revisions command", () => {

const output = stdoutWrite.mock.calls.map((c) => c[0]).join("");
expect(output).toContain("ID");
expect(output).toContain("VERSION");
expect(output).toContain("TITLE");
expect(output).toContain("AUTHOR");
expect(output).toContain("CREATED");
});

Expand Down
Loading