Skip to content

fix(web): removing a secret provider lists the Simlock tokens that read from it - #31754

Open
MarcoDotIO wants to merge 1 commit into
pingdotgg:mainfrom
MarcoDotIO:fix/secrets
Open

MarcoDotIO wants to merge 1 commit into
pingdotgg:mainfrom
MarcoDotIO:fix/secrets

Conversation

@MarcoDotIO

Copy link
Copy Markdown
Contributor

Problem

Removing a secret provider in Settings → Secrets opens a confirmation that lists every setting reading from that provider, because removal unsets them. Since #31652 a Simlock host's agent token can come from a secret provider, and on removal the server unsets it (withoutRemovedSecretProviderReferences in apps/server/src/settingsSecrets.ts covers simlockDeviceHosts through mapSettingsSecretValues). The dialog's list (secretProviderUses in apps/web/src/components/settings/SecretsSettings.logic.ts, added in #31651, which merged alongside #31652) never looked at simlockDeviceHosts. So if a Simlock token was the only thing using a provider, the dialog said "Nothing reads from …" and removal then cleared the token without warning. If other settings also used the provider, the Simlock token was left out of the list.

Repro: turn on Simlock in Labs, add a Simlock host whose agent token comes from a 1Password provider, then click Remove on that provider in Settings → Secrets.

Change

secretProviderUses now lists Simlock hosts the same way it lists EAS hosts (<host label> · Agent Token). The server's other lists of secret references (invalidateSettingsSecrets, mapSecretFieldGroups) already include Simlock hosts. This list was the only one that didn't.

Scope and approval

This is a small fix for an obvious gap: two PRs merged the same day, and the list one added didn't pick up the field the other one made referenceable. One function and its existing test change. No new behaviour. The dialog now lists everything the server already unsets.

This changes what the Remove dialog shows: one extra row when a Simlock token uses the provider.

Verification

Extended the existing SecretsSettings.logic.test.ts fixture with a Simlock host whose agent token reads from the personal provider.

Before the fix (on upstream/main):

AssertionError: expected [ Array(5) ] to deeply equal [ Array(6) ]
    "EAS Simulator · Access Token",
-   "Simlock Pool · Agent Token",
 Test Files  1 failed (1)
      Tests  1 failed | 2 passed (3)

After:

 Test Files  1 passed (1)
      Tests  3 passed (3)

vp lint and vp fmt --check on apps/web/src/components/settings pass. effect-tsgo tsrs --noEmit in apps/web reports 0 errors.

Real client: web dev server (with T3CODE_FEATURE_SECRETS=1) on empty state, with a 1Password provider "Personal 1Password" and Labs Simlock on, and a Simlock host "Simlock Pool" whose agent token reads op://Shared/simlock/token from that provider. Nothing else uses the provider. Settings → Secrets → Remove:

Before (main) After (this branch)
Remove Personal 1Password dialog saying nothing reads from it Remove Personal 1Password dialog listing Simlock Pool · Agent Token

Not checked: desktop as a separate app (same web component). Mobile has no Remove-provider dialog.

Claude Opus 5.5 (Claude Code desktop app)

🤖 Generated with Claude Code

…ad from it

The Remove dialog lists every setting that reads from the secret provider, since removing it
unsets them. A Simlock agent token can name a secret provider, and the server unsets it on
removal, but the list skipped Simlock hosts, so the dialog could say nothing reads from the
provider and then silently clear the token.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Nov 6, 2026
@macroscopeapp

macroscopeapp Bot commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at dd31e51

Macroscope's review found this PR approvable — The removal confirmation now lists Simlock token references alongside other secret-provider dependencies. This only expands the warning display; it does not expose token values or change how removal works.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

Secret provider use detection now scans SIMLock device-host settings. The test fixture verifies that a personal-provider agent token appears as a SIMLock secret use.

Changes: SIMLock secret references

Layer / File(s) Summary
Scan SIMLock settings for secret references
apps/web/src/components/settings/SecretsSettings.logic.ts, apps/web/src/components/settings/SecretsSettings.logic.test.ts
The settings input and provider-use scan now include SIMLock device hosts. The test checks the Simlock Pool · Agent Token label for a personal-provider token.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge

Merge Risk: Minimal · up to dd31e

SIMLock token references are included in the removal warning. No merge-blocking risk was identified.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title clearly describes the change to the secret provider removal warning.
Description check Passed The description covers the problem, fix, scope, and verification. It includes before-and-after screenshots and identifies the model and harness.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between 7bc2a40 and dd31e51.

Files selected for processing: 2

  • apps/web/src/components/settings/SecretsSettings.logic.test.ts
  • apps/web/src/components/settings/SecretsSettings.logic.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S 10-29 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant