Skip to content

improvement(secrets): new query parameter filterInaccessibleSecrets#5920

Open
adilsitos wants to merge 1 commit intomainfrom
feat/adilsitos/ENG-4732
Open

improvement(secrets): new query parameter filterInaccessibleSecrets#5920
adilsitos wants to merge 1 commit intomainfrom
feat/adilsitos/ENG-4732

Conversation

@adilsitos
Copy link
Copy Markdown
Contributor

Context

This query parameter filterInaccessibleSecrets will only fetch secrets the identity has access to. It modifies the secret list endpoint so it is possible to only check the secrets your identity has access (can see the value), not the one that it can describe (they exist and you can check it, but you don't have access)

Screenshots

Steps to verify the change

  • Create a new machine identity
  • Define a role and define it is only possible to describe the secrets
  • Authenticate with the machine identity using: http://localhost:8080/api/v1/auth/universal-auth/login (use universal access to be more easy)
  • Query the list endpoint
  • check that the secret was not returned
  • Modify the role to also has read access
  • Query again and check that the secret was returned.

Type

  • Fix
  • Feature
  • Improvement
  • Breaking
  • Docs
  • Chore

Checklist

  • Title follows the conventional commit format: type(scope): short description (scope is optional, e.g., fix: prevent crash on sync or fix(api): handle null response).
  • Tested locally
  • Updated docs (if needed)
  • Updated CLAUDE.md files (if needed)
  • Read the contributing guide

@linear
Copy link
Copy Markdown

linear bot commented Apr 2, 2026

@maidul98
Copy link
Copy Markdown
Collaborator

maidul98 commented Apr 2, 2026

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 2, 2026

Greptile Summary

This PR adds a filterInaccessibleSecrets query parameter to the list endpoint that, when enabled, omits entries the caller lacks ReadValue permission for rather than returning them with a hidden value.

  • The new flag is absent from the etagField key computation in the bridge service. Two calls that differ only on this flag share the same Redis ETag hash field, so a client reusing an ETag across the two modes can receive a spurious 304 Not Modified with an incorrect (or empty) payload.

Confidence Score: 4/5

  • Generally safe, but the ETag caching bug can serve stale/incorrect responses to clients that use conditional requests.
  • There is one P1 defect: filterInaccessibleSecrets is missing from the etagField key, which can produce incorrect 304 Not Modified responses when clients alternate between the two parameter values. The core filtering logic and permission checks are correct.
  • backend/src/services/secret-v2-bridge/secret-v2-bridge-service.ts — the etagField generation at line 1172 needs filterInaccessibleSecrets added.

Important Files Changed

Filename Overview
backend/src/services/secret-v2-bridge/secret-v2-bridge-service.ts Adds filterInaccessibleSecrets filtering logic to the secrets list; filterInaccessibleSecrets is missing from the ETag field key, causing potential incorrect 304 responses.
backend/src/server/routes/v4/secret-router.ts Adds filterInaccessibleSecrets query parameter to the list secrets schema and passes it to the service layer correctly.
backend/src/services/secret/secret-service.ts Passes filterInaccessibleSecrets through to the v2 bridge and correctly sets throwOnMissingReadValuePermission=false when filtering is active.

Comments Outside Diff (1)

  1. backend/src/services/secret-v2-bridge/secret-v2-bridge-service.ts, line 1172-1183 (link)

    P1 New parameter missing from ETag field key

    The filterInaccessibleSecrets flag is explicitly destructured out of dto before the ...params spread, so it is absent from etagField. Two requests that differ only by this flag resolve to the same Redis hash field. A client that first calls with the flag set to false, stores the returned ETag, then calls again with the flag set to true and passes If-None-Match, will receive a 304 Not Modified — even though the filtered response would differ.

    Add filterInaccessibleSecrets to the object passed to generateCacheKeyFromData in the etagField computation.

Reviews (1): Last reviewed commit: "add new query parameter to only fetch en..." | Re-trigger Greptile

Copy link
Copy Markdown
Collaborator

maidul98 commented Apr 2, 2026

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