fix(lns-service): disclose env var + destination domain on credential approval card (M10)#93
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses security finding M10 by enriching the lns-service credential approval UI so developers can see (a) which host env var will be read and (b) which destination domain(s) a credential may be sent to, plus a warning when the provider/integration is not part of the bundled set.
Changes:
- Thread provider disclosure fields (
env_var,injection_domains,is_project_defined) through the credential/session → window/notification → tray rendering pipeline. - Add a
CredentialInjection::domain()accessor to extract/collect destination domains for disclosure. - Wire bundled integration IDs into the credential session so the UI can distinguish bundled vs non-bundled providers.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| crates/lns-service/src/tray.rs | Renders “Reads host env”, “Sends to”, and non-bundled warning labels on credential cards. |
| crates/lns-service/src/supervisor/adapter.rs | Supplies bundled integration IDs into the credential subsystem via .with_bundled_ids(...). |
| crates/lns-service/src/credential_flow/watcher.rs | Updates test fixtures to include newly added prompt fields. |
| crates/lns-service/src/credential_flow/session.rs | Adds disclosure fields to prompts, stores bundled IDs, and derives disclosure data from provider injections. |
| crates/lns-service/src/credential_flow/notification.rs | Threads new sign-in disclosure fields into the window notifier. |
| crates/lns-service/src/approval_flow/window.rs | Carries disclosure fields into window-state prompt/card models and adds TEXT_WARN. |
| crates/lns-service/src/approval_flow/protocol.rs | Adds CredentialInjection::domain() accessor used to collect injection domains for disclosure. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
chenhunghan
approved these changes
Jun 25, 2026
chenhunghan
pushed a commit
that referenced
this pull request
Jun 25, 2026
…ate) Only used within lns-service (session.rs + same-module test); pub(crate) avoids needlessly widening the library crate's public API. Addresses Copilot review comment on PR #93.
6ac0554 to
bc89860
Compare
chenhunghan
pushed a commit
that referenced
this pull request
Jun 25, 2026
…ate) Only used within lns-service (session.rs + same-module test); pub(crate) avoids needlessly widening the library crate's public API. Addresses Copilot review comment on PR #93.
bc89860 to
b8d2b9b
Compare
chenhunghan
pushed a commit
that referenced
this pull request
Jun 25, 2026
…ate) Only used within lns-service (session.rs + same-module test); pub(crate) avoids needlessly widening the library crate's public API. Addresses Copilot review comment on PR #93.
b8d2b9b to
43f7f75
Compare
… approval card (M10) The credential approval card previously showed only the provider id and a generic action string, hiding which host env var is read and which domain the credential is sent to. Since lns-policy.yaml controls all three, a developer could be tricked into approving exfiltration. This change threads the resolved provider's env_var, injection domain(s), and a project-defined flag through to both the credential approval card and the sign-in card, rendering them as disclosure labels. Full lns-service build/test runs in CI (local sandbox lacks GTK system libs).
…fields (M10) Add env_var, injection_domains, and is_project_defined to the CredentialCardPrompt and SignInCard literals so the example compiles after the M10 credential-card disclosure changes.
… branch for 100% gate
…ate) Only used within lns-service (session.rs + same-module test); pub(crate) avoids needlessly widening the library crate's public API. Addresses Copilot review comment on PR #93.
43f7f75 to
832e612
Compare
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
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.
M10: credential approval card must disclose env var + destination domain
Problem
The credential approval card showed only the provider id and a generic action string. It hid which host env var is read and which domain the credential is sent to. Because a project's
lns-policy.yamlcontrols the provider id, the env var, and the injection domain, a developer could be socially engineered into approving a custom (project-defined) provider that exfiltrates a host secret to an attacker-controlled domain — the card gave them nothing to notice the mismatch.Fix
Thread the resolved provider's disclosure data to the approval card and render it:
Reads host env: $X— the env var the provider readsSends to: <domain[, domain]>— deduped/sorted injection domainsProject-defined provider (not built-in)— warning-colored, shown when the provider id is not in the bundled setHost-side only. No wire-protocol / guest-facing change.
Changes
protocol.rs—CredentialInjection::domain()accessor.session.rs—env_var/injection_domains/is_project_definedonCredentialPendingPrompt(+SignInPrompt);bundled_idsfield +with_bundled_idsbuilder;provider_disclosure()helper; wired throughsubmit_pending,run_pkce_connect,run_oauth_connect.window.rs— same fields onCredentialCardPrompt+SignInCard, threaded ininsert_credential_pending;TEXT_WARNcolor.notification.rs— threads fieldsSignInPrompt→SignInCard.adapter.rs— wires bundled integration ids via.with_bundled_ids(...).tray.rs— renders the three disclosure labels on the card.watcher.rs— test constructions updated.Tests
Regression tests at session / window / notifier layers:
submit_pendingthreadsenv_var+ domains for a custom provider;is_project_definedis true when the id is not bundled, false when it is;env_varstaysNonefor an unknown credential id.insert_credential_pendingcarries the fields through to the snapshot.present()passes the fields to the card.Verification note
cargo fmt --all --checkandcargo check --package lns-policypass locally. The fulllns-servicebuild + test runs in CI — it links egui/GTK, and this sandbox can't install the GTK/glib system libs, so the authoritative build/test for this crate is CI.Addresses security finding M10.