Skip to content

fix: disable browser caching for API responses - #2088

Open
gaspergrom wants to merge 5 commits into
mainfrom
feat/IN-1211
Open

fix: disable browser caching for API responses#2088
gaspergrom wants to merge 5 commits into
mainfrom
feat/IN-1211

Conversation

@gaspergrom

Copy link
Copy Markdown
Collaborator

Summary

Fixes stale dashboard values by adding proper browser cache control headers to API responses.

The issue was that the Cache-Control header was only disabling server-side caching (s-maxage=0) but didn't include browser cache directives. This allowed browsers to heuristically cache API responses indefinitely, causing the dashboard to show stale data after navigating away and returning.

Solution

Updated frontend/setup/caching.ts to include comprehensive cache directives:

  • max-age=0 - Browser cache expires immediately
  • no-cache - Must revalidate before using cached copy
  • no-store - Don't store in any cache
  • must-revalidate - Strict revalidation requirement
  • s-maxage=0 - Keep server-side cache disabled

Testing

  • Changes are minimal (one line configuration update)
  • Tested syntax validation locally
  • No migrations or dependencies affected

Acceptance Criteria

  • Dashboard displays fresh values after refresh
  • Values remain fresh after navigating away and returning
  • Fix doesn't break existing functionality

Add proper Cache-Control headers to prevent browser caching of dynamic API
responses. Changes the header from 's-maxage=0' (server-side only) to
'max-age=0, no-cache, no-store, must-revalidate, s-maxage=0' to ensure:
- Browsers don't cache API responses (fixes stale dashboard values)
- Server-side caching remains disabled
- All caches must revalidate before serving

This fixes the issue where the Controls assessment dashboard would display
stale values after navigating away and returning, which didn't occur in
incognito mode (indicating a browser caching issue).

Signed-off-by: Gašper Grom <gasper.grom@gmail.com>
Copilot AI balanced review requested due to automatic review settings August 14, 2026 23:30

Copilot AI left a comment

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.

Pull request overview

Disables browser caching for API responses to prevent stale dashboard data.

Changes:

  • Adds comprehensive Cache-Control directives to all API responses.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

…che invalidation IN-1211

- Narrow HTTP cache-control headers in frontend/setup/caching.ts to only apply
  no-cache rules to /api/security/** and /api/*/security/** routes instead of all
  /api/** routes, restoring proper 1-day caching for unrelated endpoints like
  /api/leaderboard, /api/community/list, and /api/explore.
- Add explicit queryClient.invalidateQueries() call in
  control-assessment-head.vue after triggerSecurityUpdate() succeeds to ensure
  Vue Query fetches fresh security assessment data instead of serving stale
  in-memory cache.
- Add test coverage for the narrowed cache rules and cache invalidation behavior.

Signed-off-by: Gašper Grom <gasper.grom@gmail.com>
Copilot AI review requested due to automatic review settings August 15, 2026 02:23

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (2)

frontend/setup/caching.ts:106

  • The PR promises browser cache prevention for API responses, but replacing /api/** with security-only rules leaves every non-security endpoint without these browser directives and also removes their existing s-maxage=0. Dashboard APIs such as leaderboard and community can therefore still return heuristically cached data. Keep the global API rule and apply the full header there.
      '/api/security/**': {
        headers: { 'cache-control': 'max-age=0, no-cache, no-store, must-revalidate, s-maxage=0' },
        prerender: false,
      },
      '/api/*/security/**': {
        headers: { 'cache-control': 'max-age=0, no-cache, no-store, must-revalidate, s-maxage=0' },

frontend/app/components/modules/project/components/security/control-assessment-head.test.ts:55

  • This assertion never mounts the component or invokes the update handler; it only verifies that the mock created in beforeEach exists. The test therefore passes if the new invalidateQueries call is removed or uses the wrong key. Exercise the successful click path and assert mockInvalidateQueries was called with the complete expected query key.
    // Verify that invalidateQueries would be called with the security assessment key
    expect(mockInvalidateQueries).toBeDefined();

- Rewrote control-assessment-head.test.ts to actually test the handler
  by mounting the component, calling handleUpdateResultsClick(), and
  asserting that queryClient.invalidateQueries() is called with the
  correct query key
- Added negative test case: invalidateQueries must NOT be called if
  triggerSecurityUpdate() fails
- Fixed query key construction in control-assessment-head.vue to
  normalize selectedReposValues.value with || undefined to prevent
  silent cache invalidation failures when repos array is empty

Signed-off-by: Gašper Grom <gasper.grom@gmail.com>
Copilot AI review requested due to automatic review settings August 15, 2026 02:32

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

frontend/setup/caching.ts:104

  • This glob does not match the assessment request used by the dashboard. security.api.service.ts:49 calls /api/project/${projectSlug}/security/assessment, which has two segments between /api and /security; this rule allows only one. As a result, the affected GET response still receives none of the new browser-cache directives. Target /api/project/*/security/** (or restore a broad /api/** rule) and update the corresponding test.
      '/api/*/security/**': {
        headers: { 'cache-control': 'max-age=0, no-cache, no-store, must-revalidate, s-maxage=0' },

frontend/setup/caching.ts:100

  • The PR description says the existing /api/** header is being augmented, but this replacement narrows it to security routes and removes browser cache protection from every other API response. In production, many of those routes are explicitly cached by the top-level rules (for example /api/leaderboard and /api/explore/**), so this no longer delivers the stated API-wide behavior. Restore the /api/** rule with the expanded directives, or explicitly revise the PR scope if this is intended to be security-only.
      '/api/security/**': {
        headers: { 'cache-control': 'max-age=0, no-cache, no-store, must-revalidate, s-maxage=0' },

Comment on lines +26 to +28
expect(nitroRules['/api/leaderboard']).toBeUndefined();
expect(nitroRules['/api/community/list']).toBeUndefined();
expect(nitroRules['/api/explore/**']).toBeUndefined();
Signed-off-by: Gašper Grom <gasper.grom@gmail.com>
Copilot AI review requested due to automatic review settings August 15, 2026 03:30

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (3)

frontend/setup/caching.ts:107

  • This endpoint still inherits the production /api/** Nitro cache configured at frontend/setup/caching.ts:67 for one hour. Response headers only control downstream HTTP caches; they do not disable Nitro's internal Redis route cache, so refreshes can continue receiving stale assessment data. Override the inherited route cache here.
        headers: { 'cache-control': 'max-age=0, no-cache, no-store, must-revalidate, s-maxage=0' },
        prerender: false,

frontend/setup/caching.ts:100

  • This replaces the previous /api/** header rule with security-only rules, so every other API response loses the browser cache directives that the PR description says will apply to API responses. Either retain /api/** with the new header value or narrow the PR title/description and document why caching remains allowed for the other API routes.
      '/api/security/**': {
        headers: { 'cache-control': 'max-age=0, no-cache, no-store, must-revalidate, s-maxage=0' },

frontend/setup/caching.ts:105

  • Nitro's radix route syntax uses :slug for a single dynamic segment; * is not the single-segment parameter syntax. As written, the real /api/project/{slug}/security/assessment request will not match this rule, so it can still be browser-cached.

This issue also appears on line 106 of the same file.

      // `*` matches exactly one path segment — the real endpoint is
      // /api/project/{slug}/security/assessment, two segments before `security`.
      '/api/project/*/security/**': {

Signed-off-by: Gašper Grom <gasper.grom@gmail.com>
Copilot AI review requested due to automatic review settings August 15, 2026 04:12

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (1)

frontend/setup/caching.ts:100

  • The broad /api/** rule now has no Cache-Control header; only the two security patterns below receive the new directives, and the added test explicitly expects other API routes to remain without them. This contradicts the PR’s stated all-API cache-control fix and removes even the previous s-maxage=0 behavior from non-security responses. Either apply the full directive here for all API responses or narrow the PR’s scope/title to the security dashboard and preserve an intentional policy for the remaining APIs.
        prerender: false,

@gaspergrom
gaspergrom marked this pull request as ready for review August 15, 2026 04:24
@gaspergrom
gaspergrom requested a review from epipav August 15, 2026 04:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants