Skip to content
Merged
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
40 changes: 36 additions & 4 deletions static/app/views/explore/conversations/layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,14 @@ import {Tooltip} from '@sentry/scraps/tooltip';

import Feature from 'sentry/components/acl/feature';
import {AnalyticsArea} from 'sentry/components/analyticsArea';
import {Breadcrumbs} from 'sentry/components/breadcrumbs';
import {type Crumb, Breadcrumbs} from 'sentry/components/breadcrumbs';
import {FeedbackButton} from 'sentry/components/feedbackButton/feedbackButton';
import {NoAccess} from 'sentry/components/noAccess';
import {NoProjectMessage} from 'sentry/components/noProjectMessage';
import {PageFiltersContainer} from 'sentry/components/pageFilters/container';
import {SentryDocumentTitle} from 'sentry/components/sentryDocumentTitle';
import {defined} from 'sentry/utils/defined';
import {decodeScalar} from 'sentry/utils/queryString';
import {isUUID} from 'sentry/utils/string/isUUID';
import {normalizeUrl} from 'sentry/utils/url/normalizeUrl';
import {useLocation} from 'sentry/utils/useLocation';
Expand Down Expand Up @@ -127,9 +129,7 @@ function ConversationsHeader() {
]}
/>
) : (
<Fragment>
{CONVERSATIONS_LANDING_TITLE} <FeatureBadge type="beta" />
</Fragment>
<ConversationsLandingTitle />
)}
</TopBar.Slot>
<TopBar.Slot name="feedback">
Expand Down Expand Up @@ -165,4 +165,36 @@ function useRestoredListQuery(
: undefined;
}

function ConversationsLandingTitle() {
const organization = useOrganization();
const location = useLocation();
const savedQueryTitle = decodeScalar(location.query.title);
const savedQueryId = decodeScalar(location.query.id);

if (defined(savedQueryId) && defined(savedQueryTitle) && savedQueryTitle.length > 0) {
Comment on lines +172 to +174

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 breadcrumb for a saved conversation query is rendered based on URL parameters (savedQueryId, savedQueryTitle) without validating that the query actually exists on the server.
Severity: LOW

Suggested Fix

To align with the pattern used in logs and traces, the component should use the useGetSavedQuery(savedQueryId) hook. The breadcrumb should only be rendered if the hook returns a valid savedQuery object from the API, and the breadcrumb title should be sourced from savedQuery.name instead of the URL parameter.

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: static/app/views/explore/conversations/layout.tsx#L172-L174

Potential issue: The code in `ConversationsLandingTitle` renders a breadcrumb based on
`savedQueryId` and `savedQueryTitle` from the URL query parameters. However, it only
checks for the presence of these parameters (`defined(savedQueryId)`) and not their
validity. This allows for the creation of URLs with arbitrary `id` and `title` values,
which will render a fake breadcrumb for a non-existent saved query. This behavior is
inconsistent with parallel implementations for logs and traces, which validate the ID
against the server using `useGetSavedQuery` before rendering the breadcrumb. While not a
security risk, it's incorrect UI behavior.

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

const conversationsBaseUrl = normalizeUrl(
`/organizations/${organization.slug}/explore/${CONVERSATIONS_LANDING_SUB_PATH}/`
);
const crumbs: Crumb[] = [
{
label: CONVERSATIONS_SIDEBAR_LABEL,
to: {
pathname: conversationsBaseUrl,
query: {statsPeriod: '24h'},
},
},
{
label: savedQueryTitle,
},
];
return <Breadcrumbs crumbs={crumbs} />;
}

return (
<Fragment>
{CONVERSATIONS_LANDING_TITLE} <FeatureBadge type="beta" />
</Fragment>
);
}

export default ConversationsLayout;
Loading