fix(auth): serialize legacy auth-file uploads with SIWC bind commits

- Acquire the Codex file lock around in-place disk writes in writeAuthFile when writing or replacing Codex credentials, and resolve symlinked paths in canonicalLockPath so upload and bind paths share the same lock.
- Verify in BindCodexSIWC before atomic replacement that the backing file bytes on disk still match the initial read, preventing uncooperative in-place rewrites from being silently overwritten.
- Add regression tests covering in-place rewrites during SIWC bind commits and legacy uploads contending on the Codex file lock.

Closes: #6811
This commit is contained in:
Luis Pater
2026-10-21 01:09:40 +08:00
parent 3870639096
commit 88041f0cfb
5 changed files with 364 additions and 3 deletions

View File

@@ -17,6 +17,7 @@ import (
"time"
"github.com/gin-gonic/gin"
"github.com/router-for-me/CLIProxyAPI/v8/internal/auth/codex"
"github.com/router-for-me/CLIProxyAPI/v8/internal/clienterror"
"github.com/router-for-me/CLIProxyAPI/v8/internal/util"
"github.com/router-for-me/CLIProxyAPI/v8/internal/watcher/synthesizer"
@@ -289,7 +290,7 @@ func (h *Handler) writeAuthFile(ctx context.Context, name string, data []byte) e
}
release := h.beginSIWCFileMutation(ctx, dst)
defer release()
if errWrite := os.WriteFile(dst, data, 0o600); errWrite != nil {
if errWrite := h.writeAuthFileBytes(dst, data, auth); errWrite != nil {
return fmt.Errorf("failed to write file: %w", errWrite)
}
if err := h.upsertAuthRecord(ctx, auth); err != nil {
@@ -303,6 +304,27 @@ func (h *Handler) writeAuthFile(ctx context.Context, name string, data []byte) e
return nil
}
func (h *Handler) writeAuthFileBytes(dst string, data []byte, auth *coreauth.Auth) error {
if h.shouldLockCodexAuthFile(auth) {
return codex.WithFileLock(dst, func() error {
return os.WriteFile(dst, data, 0o600)
})
}
return os.WriteFile(dst, data, 0o600)
}
func (h *Handler) shouldLockCodexAuthFile(auth *coreauth.Auth) bool {
if coreauth.IsCodexAuthRecord(auth) {
return true
}
if h != nil && h.authManager != nil && auth != nil && auth.ID != "" {
if existing, ok := h.authManager.GetByID(auth.ID); ok && coreauth.IsCodexAuthRecord(existing) {
return true
}
}
return false
}
func requestedAuthFileNamesForDelete(c *gin.Context) ([]string, error) {
if c == nil {
return nil, nil

View File

@@ -9,6 +9,7 @@ import (
"net/url"
"os"
"path/filepath"
"runtime"
"strings"
"sync"
"sync/atomic"
@@ -542,3 +543,272 @@ func TestSIWCBindRejectsReplacedAccount(t *testing.T) {
})
}
}
type siwcCommitWindowContext struct {
context.Context
errCalls atomic.Int32
onCall int32
hook func()
}
func (c *siwcCommitWindowContext) Err() error {
if c.errCalls.Add(1) == c.onCall && c.hook != nil {
c.hook()
}
return c.Context.Err()
}
type siwcUploadProgressContext struct {
context.Context
valueOnce sync.Once
valueCh chan struct{}
doneOnce sync.Once
doneCh chan struct{}
}
func (c *siwcUploadProgressContext) Value(key any) any {
c.valueOnce.Do(func() {
close(c.valueCh)
})
return c.Context.Value(key)
}
func (c *siwcUploadProgressContext) Done() <-chan struct{} {
c.doneOnce.Do(func() {
close(c.doneCh)
})
return c.Context.Done()
}
func TestSIWCBindLegacyUploadDuringCommitWindow(t *testing.T) {
f := setupSIWCFlow(t, false)
f.router.POST("/legacy", f.h.UploadAuthFile)
state, flow := f.start(t)
uploadedBody := map[string]any{
"type": "codex",
"access_token": "uploaded-outer",
"refresh_token": "uploaded-refresh",
"account_id": "account",
"note": "uploaded-during-commit",
}
rawBody, errMarshal := json.Marshal(uploadedBody)
if errMarshal != nil {
t.Fatal(errMarshal)
}
reachedMutation := make(chan struct{})
reachedUpsert := make(chan struct{})
uploadDone := make(chan *httptest.ResponseRecorder, 1)
flow.mu.Lock()
flow.ctx = &siwcCommitWindowContext{
Context: flow.ctx,
onCall: 6,
hook: func() {
go func() {
req := httptest.NewRequest(http.MethodPost, "/legacy?name=codex.json", strings.NewReader(string(rawBody)))
req.Header.Set("Content-Type", "application/json")
req = req.WithContext(&siwcUploadProgressContext{
Context: req.Context(),
valueCh: reachedMutation,
doneCh: reachedUpsert,
})
w := httptest.NewRecorder()
f.router.ServeHTTP(w, req)
uploadDone <- w
}()
<-reachedMutation
for range 500 {
select {
case <-reachedUpsert:
return
default:
runtime.Gosched()
}
}
},
}
flow.mu.Unlock()
w := siwcRequest(t, f.router, http.MethodPost, "/callback", map[string]any{"provider": "codex", "redirect_url": siwcCallbackURL(state)})
if w.Code != http.StatusOK {
t.Fatalf("callback: %d %s", w.Code, w.Body.String())
}
waitSIWCExchange(t, flow)
if status := performOAuthStatusRequest(t, f.router, state); status.Status != "ok" {
t.Fatalf("expected bind commit to finish before locked upload replaced file, got %#v", status)
}
var wUpload *httptest.ResponseRecorder
select {
case wUpload = <-uploadDone:
case <-time.After(5 * time.Second):
t.Fatal("legacy upload did not finish")
}
if wUpload.Code != http.StatusOK {
t.Fatalf("legacy upload status = %d, body = %s", wUpload.Code, wUpload.Body.String())
}
diskData, errRead := os.ReadFile(f.path)
if errRead != nil {
t.Fatal(errRead)
}
var disk map[string]any
if errJSON := json.Unmarshal(diskData, &disk); errJSON != nil {
t.Fatal(errJSON)
}
got, ok := f.manager.GetByID(f.auth.ID)
if !ok || got == nil {
t.Fatal("missing auth in manager after upload")
}
if disk["note"] != "uploaded-during-commit" || disk["access_token"] != "uploaded-outer" {
t.Fatalf("SIWC bind overwrote legacy upload on disk: %#v", disk)
}
if got.Metadata["note"] != "uploaded-during-commit" || got.Metadata["access_token"] != "uploaded-outer" {
t.Fatalf("unexpected in-memory metadata after upload: %#v", got.Metadata)
}
if disk["use_siwc"] != got.Metadata["use_siwc"] {
t.Fatalf("disk and memory disagree on use_siwc: disk=%v memory=%v", disk["use_siwc"], got.Metadata["use_siwc"])
}
}
func TestWriteAuthFileWaitsForCodexFileLock(t *testing.T) {
for _, tc := range []struct {
name string
useSymlink bool
uploadedBody map[string]any
}{
{
name: "codex_upload",
useSymlink: false,
uploadedBody: map[string]any{
"type": "codex",
"access_token": "uploaded-outer",
"refresh_token": "uploaded-refresh",
"account_id": "account",
"note": "after-lock",
},
},
{
name: "non_codex_replacing_existing_codex",
useSymlink: false,
uploadedBody: map[string]any{
"type": "claude",
"access_token": "claude-access",
"refresh_token": "claude-refresh",
"note": "replaced-provider",
},
},
{
name: "symlinked_auth_dir_alias",
useSymlink: true,
uploadedBody: map[string]any{
"type": "codex",
"access_token": "uploaded-outer",
"refresh_token": "uploaded-refresh",
"account_id": "account",
"note": "via-symlink-dir",
},
},
} {
t.Run(tc.name, func(t *testing.T) {
f := setupSIWCFlow(t, false)
f.router.POST("/legacy", f.h.UploadAuthFile)
lockPath := f.path
if tc.useSymlink {
aliasDir := filepath.Join(t.TempDir(), "auths-link")
if errSymlink := os.Symlink(filepath.Dir(f.path), aliasDir); errSymlink != nil {
t.Skipf("symlinks unavailable: %v", errSymlink)
}
f.h.cfg.AuthDir = aliasDir
if realPath, errEval := filepath.EvalSymlinks(f.path); errEval == nil {
lockPath = realPath
}
}
rawBody, errMarshal := json.Marshal(tc.uploadedBody)
if errMarshal != nil {
t.Fatal(errMarshal)
}
locked := make(chan struct{})
releaseLock := make(chan struct{})
var releaseOnce sync.Once
release := func() {
releaseOnce.Do(func() {
close(releaseLock)
})
}
t.Cleanup(release)
lockDone := make(chan error, 1)
go func() {
lockDone <- codex.WithFileLock(lockPath, func() error {
close(locked)
<-releaseLock
return nil
})
}()
<-locked
reachedMutation := make(chan struct{})
reachedUpsert := make(chan struct{})
uploadDone := make(chan *httptest.ResponseRecorder, 1)
go func() {
req := httptest.NewRequest(http.MethodPost, "/legacy?name=codex.json", strings.NewReader(string(rawBody)))
req.Header.Set("Content-Type", "application/json")
req = req.WithContext(&siwcUploadProgressContext{
Context: req.Context(),
valueCh: reachedMutation,
doneCh: reachedUpsert,
})
w := httptest.NewRecorder()
f.router.ServeHTTP(w, req)
uploadDone <- w
}()
<-reachedMutation
for range 200 {
runtime.Gosched()
}
select {
case <-reachedUpsert:
t.Fatal("writeAuthFile bypassed codex.WithFileLock and reached upsertAuthRecord while lock was held")
default:
}
diskWhileLocked, errRead := os.ReadFile(f.path)
if errRead != nil {
t.Fatal(errRead)
}
if strings.Contains(string(diskWhileLocked), tc.uploadedBody["note"].(string)) {
t.Fatalf("writeAuthFile modified disk while codex.WithFileLock was held: %s", string(diskWhileLocked))
}
release()
select {
case errLock := <-lockDone:
if errLock != nil {
t.Fatal(errLock)
}
case <-time.After(5 * time.Second):
t.Fatal("file lock holder did not finish")
}
var wUpload *httptest.ResponseRecorder
select {
case wUpload = <-uploadDone:
case <-time.After(5 * time.Second):
t.Fatal("legacy upload did not finish")
}
if wUpload.Code != http.StatusOK {
t.Fatalf("legacy upload status = %d, body = %s", wUpload.Code, wUpload.Body.String())
}
diskAfter, errReadAfter := os.ReadFile(f.path)
if errReadAfter != nil {
t.Fatal(errReadAfter)
}
if !strings.Contains(string(diskAfter), tc.uploadedBody["note"].(string)) {
t.Fatalf("expected uploaded content after lock release, got: %s", string(diskAfter))
}
})
}
}

View File

@@ -33,6 +33,11 @@ func canonicalLockPath(authFilePath string) string {
if abs, errAbs := filepath.Abs(cleaned); errAbs == nil && strings.TrimSpace(abs) != "" {
cleaned = filepath.Clean(abs)
}
if real, errEval := filepath.EvalSymlinks(cleaned); errEval == nil && strings.TrimSpace(real) != "" {
cleaned = filepath.Clean(real)
} else if parent, errParent := filepath.EvalSymlinks(filepath.Dir(cleaned)); errParent == nil && strings.TrimSpace(parent) != "" {
cleaned = filepath.Clean(filepath.Join(parent, filepath.Base(cleaned)))
}
return cleaned
}

View File

@@ -1,6 +1,7 @@
package auth
import (
"bytes"
"context"
"encoding/json"
"fmt"
@@ -85,12 +86,16 @@ func (m *Manager) BindCodexSIWC(ctx context.Context, id string, expectedEpoch ui
}
out["siwc"] = json.RawMessage(siwc)
out["use_siwc"] = true
if errCtx := ctx.Err(); errCtx != nil {
return errCtx
}
latestInfo, errStat := os.Lstat(path)
if errStat != nil || !os.SameFile(info, latestInfo) {
return fmt.Errorf("bind SIWC: backing file changed during binding")
}
if errCtx := ctx.Err(); errCtx != nil {
return errCtx
latestData, errReadLatest := os.ReadFile(path)
if errReadLatest != nil || !bytes.Equal(data, latestData) {
return fmt.Errorf("bind SIWC: backing file changed during binding")
}
if errWrite := codex.WriteJSONFileAtomic(path, out); errWrite != nil {
return fmt.Errorf("bind SIWC: persist binding: %w", errWrite)

View File

@@ -476,3 +476,62 @@ func TestBindCodexSIWCRejectsReplacedAccount(t *testing.T) {
t.Fatalf("failed binding mutated replaced account on disk: %#v", disk)
}
}
type bindCommitHookContext struct {
context.Context
errCalls int
onCall int
hook func()
}
func (c *bindCommitHookContext) Err() error {
c.errCalls++
if c.errCalls == c.onCall && c.hook != nil {
c.hook()
}
return c.Context.Err()
}
func TestBindCodexSIWCRejectsInPlaceRewriteDuringCommit(t *testing.T) {
m, base, path, hook := setupSIWCBinding(t, "auth.json")
rewritten := map[string]any{
"type": "codex",
"access_token": "uploaded-outer",
"refresh_token": "uploaded-refresh",
"account_id": "account",
"notes": "in-place-upload",
"disabled": false,
}
rewrittenBytes, errMarshal := json.Marshal(rewritten)
if errMarshal != nil {
t.Fatal(errMarshal)
}
beforeInfo, errStat := os.Lstat(path)
if errStat != nil {
t.Fatal(errStat)
}
ctx := &bindCommitHookContext{
Context: context.Background(),
onCall: 3,
hook: func() {
if errWrite := os.WriteFile(path, rewrittenBytes, 0o600); errWrite != nil {
t.Errorf("in-place write failed: %v", errWrite)
}
afterInfo, errAfter := os.Lstat(path)
if errAfter != nil || !os.SameFile(beforeInfo, afterInfo) {
t.Errorf("expected in-place write to preserve file identity, err=%v", errAfter)
}
},
}
_, errBind := m.BindCodexSIWC(ctx, base.ID, base.RegistrationEpoch, "account", bindingTokens("bound"))
if errBind == nil {
t.Fatal("binding unexpectedly succeeded and overwrote in-place file update")
}
if hook.updates != 0 {
t.Fatalf("failed binding published update: updates=%d", hook.updates)
}
disk := readBindingDisk(t, path)
if disk["notes"] != "in-place-upload" || disk["access_token"] != "uploaded-outer" || disk["use_siwc"] == true {
t.Fatalf("binding overwrote in-place file rewrite on disk: %#v", disk)
}
}