fix: encode secret keys to support '/' in paths#5614
fix: encode secret keys to support '/' in paths#5614vinodwagh07 wants to merge 2 commits intoInfisical:mainfrom
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Greptile SummaryThis PR enables forward slashes ( Key issues found:
The first two issues will cause runtime failures; the third masks these issues in the test suite. Confidence Score: 1/5
|
| tagIds | ||
| }) => { | ||
| const { data } = await apiRequest.post(`/api/v4/secrets/${secretKey}`, { | ||
| const { data } = await apiRequest.post(`/api/v4/secrets/${encodeURIComponent(secretKey)}`, { |
There was a problem hiding this comment.
POST endpoint will still reject secret names containing / after encoding
The backend v4 POST route at /:secretName uses SecretNameSchema (in backend/src/server/lib/schemas.ts:45-48) which explicitly rejects names containing /:
export const SecretNameSchema = BaseSecretNameSchema.refine(
(el) => !el.includes(":") && !el.includes("/"),
"Secret name cannot contain colon or forward slash."
);The frontend correctly encodes the secret key as %2F before sending the request, but Fastify's router URL-decodes the path parameter before schema validation. So %2F becomes / and fails the SecretNameSchema check with a 400 error.
Impact: The fix works for GET, PATCH, and DELETE operations (which use permissive validators like z.string().trim() or BaseSecretNameSchema), but creating a new secret with / in the key will still fail on the backend. This PR fixes retrieval/update/deletion for secrets that already have / in their keys (e.g., synced from AWS Parameter Store), but not new creation.
To support creation: The POST route's SecretNameSchema validation should be updated to permit / in the key, or Fastify/find-my-way should be configured to skip decoding %2F for path parameters before validation.
| const getSecretValue = async (secretKey: string) => { | ||
| const passwordSecret = await testServer.inject({ | ||
| url: `/api/v3/secrets/raw/${secretKey}`, | ||
| url: `/api/v3/secrets/raw/${encodeURIComponent(secretKey)}`, |
There was a problem hiding this comment.
Test coverage gap: Secret keys with / are never exercised
The encodeURIComponent addition is correct and defensive, but the test cases do not actually validate keys containing /. All secret names are generated via formatSqlUsername() (e.g., MYSQL_PASSWORD_ABCD1234), which only produces alphanumeric characters and underscores.
To verify the fix works for its intended purpose, add a dedicated test case where a secret key contains / (e.g., by passing a static key like "aws/parameter/store" instead of a formatted username) and assert that the secret retrieval succeeds end-to-end.
This ensures that secret names containing '/' are treated as part of the key name rather than URL path separators. - Encodes keys in frontend mutations and queries to prevent 404 errors. - Updates E2E tests to match encoded API behavior. Fixes Infisical#5598
21a67bc to
73fa07e
Compare
|
@greptile-apps I have addressed the feedback. I updated the SecretNameSchema in backend/src/server/lib/schemas.ts to allow forward slashes and added dedicated E2E test cases in secretTestCases that explicitly use keys with slashes. Please re-run the analysis. |
| const { secretName } = req.params["*"], | ||
| const { secretPath, environment, projectId } = req.query; |
There was a problem hiding this comment.
Syntax error: destructuring from a string always yields undefined
req.params["*"] is a string value (e.g., "path%2Fto%2Fsecret"), not an object with a secretName property. Attempting to destructure const { secretName } = req.params["*"] will always leave secretName as undefined, breaking the getSecretReferenceDependencyTree call that follows at line 1314.
The same error occurs at line 1367 in the secret-reference-tree handler.
| const { secretName } = req.params["*"], | |
| const { secretPath, environment, projectId } = req.query; | |
| const secretName = req.params["*"]; | |
| const { secretPath, environment, projectId } = req.query; |
| key: encodeURIComponent("path/to/secret"), | ||
| value: "slash-value", | ||
| comment: "Testing forward slashes in keys" | ||
| } |
There was a problem hiding this comment.
Test encodes the key at parse time, creating a literal percent-encoded string
This test stores encodeURIComponent("path/to/secret"), which evaluates to the literal string "path%2Fto%2Fsecret" at test definition time. The test then passes this already-encoded string into the URL without further encoding. The round-trip succeeds because both create and retrieve operations use the same encoded key.
However, the actual use case — a secret key containing a real forward slash character — is never tested. To verify the feature works, the test data should use the raw slash-containing key, and the createSecret helper should apply URL encoding when constructing the endpoint URL.
Description
This PR addresses issue #5598 where secret keys containing forward slashes (
/) caused 404 errors because the character was interpreted as a URL path separator.Changes
encodeURIComponentto secret keys in frontend queries and mutations to ensure they are passed as a single string (e.g.,test%2Fcred).Related Issues
Fixes #5598