Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Contributor
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
3 tasks
| "client_id": {auth.DefaultClientID}, | ||
| "subject_token": {session.AccessToken}, | ||
| "subject_token_type": {"urn:ietf:params:oauth:token-type:access_token"}, | ||
| "resource": {entry.Provider}, |
There was a problem hiding this comment.
🟡 entry.Resource is parsed from credential template but silently ignored in token exchange
The credential template parser (internal/credential/credential.go:59) supports a provider/resource syntax (e.g., {{kontext:github/readonly}}), populating entry.Resource with the sub-resource specifier. However, exchangeCredential only sends entry.Provider in the resource form field at line 207 and completely ignores entry.Resource. This means if a user specifies {{kontext:github/readonly}}, the readonly scope is silently dropped from the token exchange request, and the returned token may have broader permissions than intended.
Prompt for agents
The credential template parser in internal/credential/credential.go supports provider/resource syntax (e.g., github/readonly), storing the resource part in entry.Resource. But exchangeCredential in internal/run/run.go:207 only uses entry.Provider as the resource form field and completely ignores entry.Resource.
Possible approaches:
1. Include entry.Resource as an additional form parameter (e.g., scope) in the token exchange request.
2. Combine provider and resource into the resource field, e.g., entry.Provider + "/" + entry.Resource when entry.Resource is non-empty.
3. If the server doesn't support resource sub-specifiers yet, add a warning log when entry.Resource is non-empty so users know it's being ignored.
The right approach depends on what the server-side token exchange endpoint supports.
Was this helpful? React with 👍 or 👎 to provide feedback.
Contributor
Author
tumberger
changed the base branch from
feat/governance-telemetry-pipeline
to
graphite-base/7
Squashed PR #7 + #13 onto main after resolving squash-merge divergence. Changes from PR #7 (credential exchange): - Replace stub exchangeCredential with real RFC 8693 token exchange - Use session.IssuerURL for endpoint discovery (not hardcoded default) - Add Bearer auth header to token exchange requests - Fix filterArgs to handle --flag=value syntax (security: prevents --settings=evil.json from bypassing the governance hooks filter) - Add truncateID helper for safe session ID display Changes from PR #13 (token refresh): - Replace static access token with TokenSource function - Auto-refresh OIDC token during long-running sessions - Resolve credentials before starting sidecar (data race fix) Addresses Devin review findings on PR #7. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
tumberger
force-pushed
the
04-06-feat_wire_up_credential_exchange_via_rfc_8693_token_exchange
branch
from
f4b56ab to
3df8073
Compare
michiosw
deleted the
04-06-feat_wire_up_credential_exchange_via_rfc_8693_token_exchange
branch
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
1 participant
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
DiscoverEndpointsandOAuthMetadatafrom the auth package sorun.gocan call OAuth discoveryexchangeCredentialstub with a real implementation that callsPOST /oauth2/tokenusing RFC 8693 token exchangesubject_token— no client secret needed (public client)access_tokenfield)Note
Based on
feat/governance-telemetry-pipeline— the credential exchange importsbackend,sidecar, and proto types introduced there. Should be merged after that branch lands on main.Test plan
kontext startwith a.env.kontextcontainingGITHUB_TOKEN={{kontext:github}}resolves credentials after backend gate is relaxed (kontext-dev/kontext#410)Depends on
🤖 Generated with Claude Code