Skip to content

fix(oauth): keep Devin migration account-bound - #534

Open
luvs01 wants to merge 31 commits into
devfrom
codex/fix-cross-account-credential-binding-vulnerability
Open

luvs01 wants to merge 31 commits into
devfrom
codex/fix-cross-account-credential-binding-vulnerability

Conversation

@luvs01

@luvs01 luvs01 commented Sep 14, 2026

Copy link
Copy Markdown
Owner

Motivation

  • The startup migration for merging devin-clidevin previously moved the provider row and the OAuth credential slot independently, which can produce a cross-account binding when one destination slot is already occupied or a partial state exists.
  • The change prevents a migrated provider row from being paired with another account's retained devin credential, closing that cross-account credential-injection attack surface.

Description

  • Inspect both the configuration source/destination and the auth.json source/destination slots before making any persisted change and refuse the migration when the canonical destination credential slot is already occupied.
  • Snapshot both account-bound files (config.json and auth.json) before persisting either half so backups exist for coordinated rollbacks, while preserving the synchronous startup contract and the detached rekey semantics.
  • Adjust the detached rekey flow to avoid independently rekeying credentials when the config-side projection refused.
  • Add regression tests covering destination auth collisions and config collisions, and update structure/runtime docs to record the coordinated migration contract.

Testing

  • Ran the focused migration suite with bun test tests/providers/devin-provider-merge-migration.test.ts, which passed (18/18).
  • Ran static checks: bun run typecheck, bun run structure:check, and bun run privacy:scan; all succeeded.
  • Executed the repository full test command bun run test — the focused migration tests and static checks passed, while unrelated environment/concurrency failures appeared in parallel combo-management and Unix shim tests; these failures are orthogonal to the migration changes and do not affect the added regression coverage for the Devin migration.

Codex Task

lidge-jun and others added 30 commits September 6, 2026 01:08
…43-01a07240

[WRONG BRANCH] chore(release): promote verified candidate to 2.43.0
…in-07c0

chore(release): promote validated 2.44.0 to main
Promote frozen dev source cf9f662; no new runtime changes. Candidate CI34061274315 and service34061276621 are the validation references. Publication waits for successful validation and the final main push CI at the exact release SHA.
Promote frozen dev source cf9f662 as 2.45.0.

The repository owner explicitly authorized this main/preview release promotion and admin PR-only merge. This is a release-specific owner decision, not an independent approval or the dev-only maintainer exception.

Frozen candidate full CI34061274315 passed all25jobs after one unchanged-source rerun of Windows5; the initial holder busy assertion remains recorded without a root-cause resolution claim. Service lifecycle34061276621 passed Linux/macOS/Windows. Dev version pre-move3812 is merged. Publication still requires this actual main merge SHA's own successful push CI and Service lifecycle. No local suites were run.
[WRONG BRANCH] chore(release): promote verified 2.46.0 to main
[WRONG BRANCH] release: promote 2.47.0 to main
[WRONG BRANCH] release: apply final roster correction to main
[WRONG BRANCH] release: promote 2.48.0 to main
…in-01a08498

release: promote verified 2.49.0 product tree to main
Promotes the audited freeze SHA 12c248f unchanged. The resulting tree is byte-identical to that commit's tree, d8f5a71, so what ships is exactly what the six-lane regression audit read.
…in-01a08a81

[WRONG BRANCH] release: promote verified 2.50.0 product tree to main
[WRONG BRANCH] release: promote verified 2.51.0 product tree to main
Product tree is dev at 7bfb2ad. Only package.json differs, carrying the
stable channel version 2.52.0.

This promotion follows a CI repair and a regression sweep: dev had drifted 79
commits past its last finished run and the one run allowed to complete had
failed. lidge-jun#4390 fixed an integration test that never denied anything and had been
red on dev since lidge-jun#4342; lidge-jun#4396/lidge-jun#4397/lidge-jun#4398 closed four gaps in the quota
avoidance contract lidge-jun#4368 introduced; lidge-jun#4403 made a port fixture deterministic.
release: promote verified 2.52.0 product tree to main
Product tree is dev at eb81eaa, byte-identical (dev already carries the
2.53.0 version line).

This promotion follows a 364-commit regression audit (origin/main..981b53e)
by 15 parallel subagent lanes plus two audit-spawned fixes reviewed and merged
to dev: lidge-jun#4496 (live sideband connect watchdog) and lidge-jun#4497 (chat image part
shapes). Zero unresolved P0/P1 at promotion time. Exact-head hosted CI green
on eb81eaa (run 34750934849). Local suite NOT RUN per task rule; hosted
exact-head CI is the gate.
[WRONG BRANCH] release: promote verified 2.53.0 product tree to main
Product tree is dev at a84e6e8, byte-identical (dev already carries the
2.54.0 version line).

Headed by the Codex model-picker incident fix (lidge-jun#4531). On Codex
0.154.0-alpha.6.2 a paginated-history preflight vetoed the entire Codex config
write, so model_catalog_json never reached config.toml and both the Codex app
and the CLI fell back to their six built-in models, while ocx sync reported that
failure as success. The refusal is now scoped to the conversation-history
relabel unit on the apply direction only, narrowed to the one permanent reason,
and a provider table the home already published survives the write.

Exact-head hosted CI green on a84e6e8 via lidge-jun#4531 (run 34772128322): all four
Linux test shards, both macOS shards, gates, hygiene, storage policy, api usage,
keyring x3, npm-global x3, docker smoke, react-doctor. Local suite NOT RUN per
task rule; hosted exact-head CI is the gate.
[WRONG BRANCH] release: promote verified 2.54.0 product tree to main
@coderabbitai

coderabbitai Bot commented Sep 14, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: f7f541b1-645d-4b26-a381-0463d10ba9f4


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review 🔄 Running since 2026-09-14T02:16:01.179654Z e3cca96 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

Copy link
Copy Markdown

Deterministic PR hygiene checks passed.

@github-actions github-actions Bot changed the title fix(oauth): keep Devin migration account-bound [WRONG BRANCH] fix(oauth): keep Devin migration account-bound Sep 14, 2026
@github-actions

github-actions Bot commented Sep 14, 2026

Copy link
Copy Markdown

✅ READY

  • all PR quality gates passed.

Hygiene

Deterministic PR hygiene checks passed.

@github-actions
github-actions Bot marked this pull request as draft September 14, 2026 02:16
@github-actions github-actions Bot added the bug Something isn't working label Sep 14, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e3cca96481

ℹ️ 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".

// Warnings are emitted even on a no-op: the collision case IS the warning.
const hasSourceConfig = config.providers?.[FROM_ID] !== undefined;
const hasSourceAuth = deps.hasAuthSlot(FROM_ID);
const hasDestinationAuth = deps.hasAuthSlot(TO_ID);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Make the destination check atomic with the rekey

When another process or an already queued OAuth mutation creates auth.json["devin"] after this synchronous peek but before the detached rekeyProviderCredentials acquires the auth-store lock, hasDestinationAuth is stale and the config is saved under devin; the rekey then returns conflict and leaves both credential slots. Because createDevinAdapter resolves credentials using the configured provider ID verbatim, subsequent requests use the newly inserted devin account, recreating the cross-account binding this change is intended to prevent. Reserve or move the destination under the auth-store lock before committing the config, or defer the config commit until the locked rekey succeeds.

AGENTS.md reference: AGENTS.md:L366-L372

Useful? React with 👍 / 👎.

@luvs01 luvs01 changed the title [WRONG BRANCH] fix(oauth): keep Devin migration account-bound fix(oauth): keep Devin migration account-bound Sep 16, 2026
@luvs01
luvs01 changed the base branch from main to dev September 16, 2026 10:01
@github-actions
github-actions Bot marked this pull request as ready for review September 16, 2026 11:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

aardvark bug Something isn't working codex

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants