feat: handle single key secret stores #812
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: nishantmunjal7 <[email protected]>
nishantmunjal7
requested review from
AtMrun,
firecast and
inishchith
as code owners
📜 Docstring Coverage ReportRESULT: PASSED (minimum: 30.0%, actual: 75.5%) Detailed Coverage Report |
📦 Trivy Vulnerability Scan Results
Report Summary
Scan Result Details✅ No vulnerabilities found during the scan for |
📦 Trivy Secret Scan Results
Report Summary
Scan Result Details✅ No secrets found during the scan for |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Collaborator
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified FilesNo covered modified files...
|
|
🛠 Full Test Coverage Report: https://k.atlan.dev/coverage/application-sdk/pr/812 |
louisnow
reviewed
Nov 6, 2025
|
🛠 Docs available at: https://k.atlan.dev/application-sdk/feat-single-key-stores |
inishchith
requested changes
Nov 7, 2025
Member
inishchith
left a comment
inishchith
left a comment
There was a problem hiding this comment.
looks fine overall, bunch of nits.
post fixing those, can we add unit tests for the change?
📦 Example workflows test results
|
📦 Example workflows test results
|
📦 Example workflows test results
|
inishchith
reviewed
Nov 8, 2025
inishchith
reviewed
Nov 8, 2025
inishchith
reviewed
Nov 8, 2025
inishchith
reviewed
Nov 8, 2025
inishchith
reviewed
Nov 8, 2025
inishchith
reviewed
Nov 10, 2025
inishchith
previously approved these changes
Nov 10, 2025
There was a problem hiding this comment.
Bug: Nested secret data fails to resolve.
The apply_secret_values method doesn't check secret_data.get("extra", {}) as a fallback when resolving values in the extra field, unlike resolve_credentials which has this nested lookup at line 221. This inconsistency means apply_secret_values can't resolve secret references that are stored in a nested extra structure within secret_data, potentially causing credential resolution failures when the two methods are used interchangeably.
application_sdk/services/secretstore.py#L397-L401
application-sdk/application_sdk/services/secretstore.py
Lines 397 to 401 in f4beb89
inishchith
approved these changes
Nov 10, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changelog
Additional context (e.g. screenshots, logs, links)
Checklist
Copyleft License Compliance
Note
Introduce mode-based secret retrieval with per-field single-key lookup, enhanced secret parsing, and comprehensive tests.
application_sdk/services/secretstore.py):MULTI_KEYvsSINGLE_KEYbased oncredentialSourceandsecret-pathusing new enumsCredentialSourceandSecretMode._fetch_single_key_secretsto fetch per-field secrets (includingextra), preserving falsy values and skipping empty/nulls._process_secret_datato delegate single-key handling to new_handle_single_key_secret(parses JSON dicts, otherwise returns{key: value}).get_credentials, merge forDIRECTor resolve viaresolve_credentialsfor others; improved error logging.tests/unit/common/test_credential_utils.py):_handle_single_key_secret,_fetch_single_key_secrets, andget_credentialsmode determination, extra fields, error handling, and falsy value preservation.Written by Cursor Bugbot for commit 8f83c04. This will update automatically on new commits. Configure here.