Skip to content

Commit dbe0a69

Browse files
committed
SEP-1844: Gate the snippet execute form on the capability, not the query flag
Address review feedback. The snippet accordion withheld its execute form from a read-only session by disabling the schema query, but a disabled query still serves a cached entry. The schema is held with `staleTime: Infinity` under a key carrying no identity, and the shell never clears the query cache, so an admin's fetch would render the form for a non-admin opening the same snippet later in the same tab. The render now gates on `canMutate` as well; the query flag stays as the request optimization. Covered by a test that populates the cache as an admin and re-renders read-only, which fails without the render gate. Also restores two e2e session fixtures to non-admin. The snippet-download spec exercises a GET that stays permitted, and the sidebar-navigation specs only navigate and assert readable sentinels, so leaving both non-admin keeps end-to-end coverage of the read-only experience this change creates rather than broadening privileges the specs never use.
1 parent f20e939 commit dbe0a69

4 files changed

Lines changed: 44 additions & 8 deletions

File tree

frontend/packages/e2e/tests/sidebar-navigation.spec.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,10 @@ const MOCK_USER = {
5959
email: 'smoke@percona.com',
6060
firstName: 'Smoke',
6161
lastName: 'Test',
62-
// Admin: the app pages under test render their create / execute / delete
63-
// controls only for a session that may mutate (SEP-1844).
64-
isAdmin: true,
62+
// Deliberately non-admin: these specs only navigate and assert readable
63+
// sentinels, so they double as cross-app smoke coverage of the read-only
64+
// experience (SEP-1844).
65+
isAdmin: false,
6566
};
6667

6768
// Heading served for schema-driven apps whose display name we don't assert

frontend/packages/e2e/tests/snippets-download.spec.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,10 @@ const MOCK_USER = {
3434
email: 'smoke@percona.com',
3535
firstName: 'Smoke',
3636
lastName: 'Test',
37-
// Admin: the app pages under test render their create / execute / delete
38-
// controls only for a session that may mutate (SEP-1844).
39-
isAdmin: true,
37+
// Deliberately non-admin: snippet download is a GET, still permitted for a
38+
// read-only session, so this keeps end-to-end proof that the read path works
39+
// for exactly the users whose write controls are hidden (SEP-1844).
40+
isAdmin: false,
4041
};
4142

4243
const MOCK_SNIPPET_SCHEMA = {

frontend/packages/framework/src/components/SnippetExecutionAccordion/SnippetExecutionAccordion.test.tsx

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -528,6 +528,35 @@ describe('SnippetExecutionAccordion — write access', () => {
528528
expect(screen.queryByTestId('snippet-execute-read-only')).not.toBeInTheDocument();
529529
});
530530

531+
it('renders no execute form for a non-admin even when the schema is already cached', async () => {
532+
// A disabled query still serves a cached entry, and this schema is held with
533+
// `staleTime: Infinity` under a key that carries no identity — so an admin's
534+
// fetch must not render the form for a non-admin in the same tab.
535+
mockedApi.get.mockResolvedValue({ data: makeSchema() });
536+
const queryClient = new QueryClient({
537+
defaultOptions: { queries: { retry: false, staleTime: Infinity } },
538+
});
539+
const ui = (
540+
<SnippetExecutionAccordion snippetFilename="check.sh" executorHost="db1" defaultExpanded />
541+
);
542+
543+
// Admin populates the cache.
544+
const { unmount } = render(
545+
<QueryClientProvider client={queryClient}>{ui}</QueryClientProvider>,
546+
);
547+
await waitFor(() => {
548+
expect(screen.getByRole('button', { name: 'Execute' })).toBeInTheDocument();
549+
});
550+
unmount();
551+
552+
// Same QueryClient, now a read-only session.
553+
mockCanMutate = false;
554+
render(<QueryClientProvider client={queryClient}>{ui}</QueryClientProvider>);
555+
556+
expect(screen.getByTestId('snippet-execute-read-only')).toBeInTheDocument();
557+
expect(screen.queryByRole('button', { name: 'Execute' })).not.toBeInTheDocument();
558+
});
559+
531560
it('renders no execute form for a non-admin and fetches no schema', async () => {
532561
mockCanMutate = false;
533562
mockedApi.get.mockResolvedValue({ data: makeSchema() });

frontend/packages/framework/src/components/SnippetExecutionAccordion/SnippetExecutionAccordion.tsx

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,12 @@ export function SnippetExecutionAccordion({
140140
const [logsEntry, setLogsEntry] = useState<TaskHistoryEntry | null>(null);
141141

142142
// The form is the execute control, so a read-only session never renders it —
143-
// and never needs its schema. History and logs stay readable.
143+
// and never needs its schema. Disabling the query is only the request
144+
// optimization: react-query still serves a cached entry, and this schema is
145+
// held with `staleTime: Infinity` under a key that carries no identity, so an
146+
// admin's fetch would otherwise render the form for a non-admin reaching the
147+
// same snippet later in the same tab. The render gates on `canMutate` too.
148+
// History and logs stay readable.
144149
const schemaQuery = useSnippetAccordionSchema(snippetFilename, expanded && canMutate);
145150
const executionMutation = useSnippetAccordionExecution(snippetFilename);
146151
const historyQuery = useSnippetAccordionHistory(snippetFilename, showHistory);
@@ -225,7 +230,7 @@ export function SnippetExecutionAccordion({
225230
</Alert>
226231
)}
227232

228-
{schemaQuery.data && (
233+
{canMutate && schemaQuery.data && (
229234
<SchemaFormRenderer
230235
sections={filteredSections}
231236
onSubmit={handleSubmit}

0 commit comments

Comments
 (0)