feat: propagate credential metadata - #2305
Merged
Merged
Conversation
github-actions
Bot
added
the
size/M
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughCredential providers now classify resolved tokens by source. Client, identity, and MCP request paths propagate this metadata through contexts. Risk-control transport injects the source header for authorized origins and removes it when restricted headers are disabled. ChangesCredential source propagation
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
internal/cmdutil/transport_test.go (1)
111-111: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a resolved CredentialProvider transport regression test.
All cited tests pass
nil. The existing risk-control test uses a static stub and does not exercise the SDK transport. Resolve a token on a non-nilCredentialProvider, pass it tobuildSDKTransportWithBase, and assertX-Agent-Credential-Sourceat the final network boundary.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/cmdutil/transport_test.go` at line 111, Update the transport regression test around buildSDKTransport(nil, nil) to use a non-nil CredentialProvider, resolve a token, and construct the transport through buildSDKTransportWithBase. Assert at the final network boundary that the request contains the expected X-Agent-Credential-Source value, while preserving the existing test setup and assertions.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/riskcontrol/transport.go`:
- Around line 42-49: Restore NewTransport to its existing two-parameter
signature so current callers remain compatible, and provide CredentialSource
through a separate constructor or option without changing the default behavior.
Update the Transport initialization flow to use the new credential configuration
path while preserving existing NewTransport callers.
---
Nitpick comments:
In `@internal/cmdutil/transport_test.go`:
- Line 111: Update the transport regression test around buildSDKTransport(nil,
nil) to use a non-nil CredentialProvider, resolve a token, and construct the
transport through buildSDKTransportWithBase. Assert at the final network
boundary that the request contains the expected X-Agent-Credential-Source value,
while preserving the existing test setup and assertions.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fc82dd60-f3ab-4378-83a4-5145fa056782
📒 Files selected for processing (6)
internal/cmdutil/factory_default.gointernal/cmdutil/transport_test.gointernal/credential/credential_provider.gointernal/credential/credential_provider_test.gointernal/riskcontrol/transport.gointernal/riskcontrol/transport_test.go
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@ba21604bf4bf66268f92f344e63db93ec5198616🧩 Skill updatenpx skills add larksuite/cli#feat/credential-source-header -y -g |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2305 +/- ##
==========================================
- Coverage 76.44% 76.44% -0.01%
==========================================
Files 1025 1025
Lines 113710 113748 +38
==========================================
+ Hits 86931 86958 +27
- Misses 20108 20117 +9
- Partials 6671 6673 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
kiraWangRuilong
force-pushed
the
feat/credential-source-header
branch
from
August 11, 2026 13:43
839b04f to
f2b7168
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
kiraWangRuilong
force-pushed
the
feat/credential-source-header
branch
from
August 12, 2026 06:51
f2b7168 to
f0dafec
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/client/client.go`:
- Around line 50-67: Update APIClient.resolveAccessToken to check whether result
is nil before reading result.Token, returning newTokenMissingError(as, nil) for
a nil result or empty token. Add a regression test covering a credential
resolver that returns nil, nil and verify it produces the typed token-missing
error without panicking.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4e2391ab-ec31-4c5c-ae04-47c7ba92cf6a
📒 Files selected for processing (12)
internal/client/client.gointernal/core/types.gointernal/core/types_test.gointernal/credential/credential_provider.gointernal/credential/credential_provider_test.gointernal/credential/integration_test.gointernal/credential/types.gointernal/identitydiag/diagnostics.gointernal/riskcontrol/transport.gointernal/riskcontrol/transport_test.goshortcuts/common/mcp_client.goshortcuts/common/runner.go
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/credential/credential_provider_test.go
kiraWangRuilong
force-pushed
the
feat/credential-source-header
branch
from
August 12, 2026 07:50
f0dafec to
9a4875c
Compare
kiraWangRuilong
changed the title
kiraWangRuilong
force-pushed
the
feat/credential-source-header
branch
from
August 12, 2026 10:04
9a4875c to
4504ac5
Compare
kiraWangRuilong
force-pushed
the
feat/credential-source-header
branch
from
August 12, 2026 10:06
4504ac5 to
ba21604
Compare
liangshuo-1
approved these changes
Aug 13, 2026
Merged
3 tasks
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
Propagate request-scoped credential metadata through shared request paths.
Changes
Test Plan
Related Issues