fix(drivers): keep OAuth credentials across restarts - #1108
Conversation
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_ee42340f-bd4a-44fa-b2b4-4faa1e83ea86) |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_5974fc5a-60e4-4272-a306-d9c8e9a27c62) |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b16f803b90
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Rotations keep their own KV rows so they do not apply the whole config | ||
| // or restart the driver that just refreshed its credential. | ||
| driverSecretKey := func(driverName, key string) string { | ||
| return "driver_secret:" + driverName + ":" + key |
There was a problem hiding this comment.
When a managed OAuth driver rotates token A to B and the operator later renames the YAML driver, this key changes even though the device does not. Because the source config intentionally remains at A, the next initialization misses B and presents the now-invalidated token, disconnecting the driver; reusing an old name can conversely apply that credential to the wrong device. Store and migrate the override under a stable hardware/account identity rather than cfg.Name.
AGENTS.md reference: AGENTS.md:L38-L38
Useful? React with 👍 / 👎.
| if repo == nil && m.betaRepo.ID != "" && installed.RepoID == m.betaRepo.ID { | ||
| repo = &m.betaRepo |
There was a problem hiding this comment.
officialBetaRepository derives betaRepo.ID from the current configured repository IDs, so it can change between installation and restart. If a beta was installed under a suffixed ID because of a collision and that colliding repository is later removed, installed.RepoID matches neither the configured repositories nor the newly derived beta ID here, causing RuntimePolicy to return nil and the signed read-only artifact to run with the unrestricted legacy policy. The reverse change can bind an existing beta install to an unrelated configured repository and fail verification; recognize the immutable trust identity recorded at installation instead of only the current generated ID.
Useful? React with 👍 / 👎.
1 participant
After myUplink rotates token A to B, Core can restart the driver with A even when B is saved in SQLite. The initial driver loop ran before the secret callbacks were installed. myUplink also refreshes during
driver_init, so that first rotation could not be saved.Create the registry with both secret callbacks before any driver starts. Signed read-only OAuth drivers can save only keys declared in their verified metadata, in their own driver namespace. Keys have a fixed format and length; values are capped at 1 MiB. Control-v2 drivers receive no secret-persistence capability.
The signed myUplink manifest declares
http.postand one auth path, but policy validation previously rejected that combination. Admit it only with read-only,http.getand the declared auth path. General writes remain denied, and auth POSTs cannot follow redirects. Official beta installs now resolve the same signed runtime policy through their existing beta trust source, even when Settings lists only the stable repository.Validation:
make verifypasses on the final source, including the fixed daytime native fixture from test(forecast): keep PV restore fixtures in daylight #1107.No Lua artifact, driver pin or manifest schema changes. The overlapping open PRs #734, #735, #826, #1003 and #1052 touch other sections of
main.go; this change leaves their work in place. Driver-host ownership remains with the repository owner under the delegated beta release process.Note
High Risk
Changes driver credential persistence, OAuth HTTP sandboxing, and signed runtime policy for managed/beta drivers—security-sensitive paths where a bug could leak or overwrite secrets or widen network write access.
Overview
Fixes myUplink-style OAuth where Core could restart with a stale
refresh_tokenfrom config even after a rotated token was saved in SQLite, because secret callbacks were wired after the first driverAdd(including rotations duringdriver_init).Startup wiring:
newDriverRegistryinstallsSecretPersister/SecretOverride(per-driverdriver_secret:<name>:<key>KV in state) before any driver init or poll. Registry comments now require this ordering.Managed read-only OAuth: Signed manifests can grant
http.postonly withhttp.getand a declaredAuthPostPath;ConfigSecretslists which keyshost.persist_secretmay write. Keys are validated (length/format); values cap at 1 MiB. Control-v2 / non-OAuth managed drivers still cannot persist secrets. OAuth token POSTs must hit the exact path (no suffix/traversal) and cannot follow redirects (307/308).Beta installs:
RuntimePolicyresolves installs from the pinned beta trust source even when Settings lists only stable, with tests for cache reload, unknown repo, and bad signatures.Reviewed by Cursor Bugbot for commit ab21200. Bugbot is set up for automated code reviews on this repo. Configure here.