protondrive: test config obscuring the password and refused updates

The password test now runs rclone config with the password in plain
text and checks it is stored obscured; a test checks a file whose key
isn't its owner's can't be updated; and the log test checks every
session token the fake issued, which it now labels by kind, not only
the saved ones.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

backend/protondrive/protontest/auth.go

@@ -239,7 +239,7 @@ func (s *Server) NewSession(u *User) Session {
 // newSessionLocked creates a session for u. s.mu must be held.
 func (s *Server) newSessionLocked(u *User, locked bool) *session {
 	sess := &session{
-		Session: Session{UID: randomID(), AccessToken: s.newSecretLocked(), RefreshToken: s.newSecretLocked()},
+		Session: Session{UID: randomID(), AccessToken: s.newSecretLocked(SessionToken), RefreshToken: s.newSecretLocked(SessionToken)},
 		user:    u,
 		locked:  locked,
 	}
@@ -344,8 +344,8 @@ func (s *Server) handleRefresh(w http.ResponseWriter, r *http.Request) {
 		writeError(w, http.StatusUnprocessableEntity, api.CodeRefreshTokenInvalid, "protontest: invalid refresh token")
 		return
 	}
-	sess.AccessToken = s.newSecretLocked()
-	sess.RefreshToken = s.newSecretLocked()
+	sess.AccessToken = s.newSecretLocked(SessionToken)
+	sess.RefreshToken = s.newSecretLocked(SessionToken)
 	sess.expired = false
 	writeJSON(w, http.StatusOK, &api.AuthRefreshResponse{
 		Code:         api.CodeOK,
@@ -498,7 +498,7 @@ func (s *Server) handleKeySalts(w http.ResponseWriter, r *http.Request, u *User)
 func (s *Server) handleForkInit(w http.ResponseWriter, r *http.Request) {
 	f := &fork{userCode: strings.ToUpper(hex.EncodeToString(randomBytes(4)))}
 	s.mu.Lock()
-	selector := s.newSecretLocked()
+	selector := s.newSecretLocked(ForkSelector)
 	s.forks[selector] = f
 	s.mu.Unlock()
 	writeJSON(w, http.StatusOK, &api.ForkInitResponse{Code: api.CodeOK, Selector: selector, UserCode: f.userCode})

backend/protondrive/protontest/drive.go

@@ -1675,7 +1675,7 @@ func (s *Server) handleRevision(w http.ResponseWriter, r *http.Request, u *User,
 			SignatureEmail: nullable(b.signatureEmail),
 		}
 		if !noURLs {
-			id, token := randomID(), s.newSecretLocked()
+			id, token := randomID(), s.newSecretLocked(StorageToken)
 			s.downloads[id] = &download{token: token, block: b}
 			bareURL := s.URL() + "/storage/blocks/" + id
 			bj.Token, bj.BareURL = &token, &bareURL

backend/protondrive/protontest/drive_test.go

@@ -636,7 +636,11 @@ func TestIssuedSecrets(t *testing.T) {
 	var rev revisionResponse
 	status, body := getJSON(t, s, sess, "/drive/v2/volumes/"+u.VolumeID()+"/files/"+file+"/revisions/"+u.view().d.links[file].activeRevision().id, &rev)
 	require.Equal(t, http.StatusOK, status, "body: %s", body)
-	assert.Contains(t, s.IssuedSecrets(), *rev.Revision.Blocks[0].Token)
+	token := *rev.Revision.Blocks[0].Token
+	assert.Equal(t, []string{token}, s.IssuedSecrets(StorageToken))
+	assert.Equal(t, []string{sess.AccessToken, sess.RefreshToken}, s.IssuedSecrets(SessionToken))
+	assert.Equal(t, []string{sess.AccessToken, sess.RefreshToken, token}, s.IssuedSecrets())
+	assert.Empty(t, s.IssuedSecrets(ForkSelector))
 }
 
 func TestStoredFileRoundTrip(t *testing.T) {

backend/protondrive/protontest/server.go

@@ -85,7 +85,7 @@ type Server struct {
 	uploads          map[string]*uploadURL  // block upload URLs by ID
 	storageUploads   int                    // block uploads in progress
 	storagePeak      int                    // most block uploads in progress at once
-	secrets          []string               // every secret issued, in order
+	secrets          []issuedSecret         // every secret issued, in order
 }
 
 // New starts a fake server which is closed when the test ends.
@@ -175,21 +175,42 @@ func (s *Server) ResetRequests() {
 	s.storagePeak = s.storageUploads
 }
 
-// newSecretLocked returns a new random secret and records it for
-// IssuedSecrets. s.mu must be held.
-func (s *Server) newSecretLocked() string {
+// SecretKind is a kind of secret the server issues.
+type SecretKind int
+
+// Kinds of secret the server issues.
+const (
+	SessionToken SecretKind = iota + 1 // an access or refresh token
+	ForkSelector                       // the selector polling a session fork
+	StorageToken                       // a block upload or download token
+)
+
+// issuedSecret is a secret the server handed to a client.
+type issuedSecret struct {
+	kind  SecretKind
+	value string
+}
+
+// newSecretLocked returns a new random secret of kind and records it
+// for IssuedSecrets. s.mu must be held.
+func (s *Server) newSecretLocked(kind SecretKind) string {
 	secret := randomID()
-	s.secrets = append(s.secrets, secret)
+	s.secrets = append(s.secrets, issuedSecret{kind: kind, value: secret})
 	return secret
 }
 
-// IssuedSecrets returns every secret the server has handed to a
-// client: session access and refresh tokens, fork selectors, and block
-// upload and download tokens.
-func (s *Server) IssuedSecrets() []string {
+// IssuedSecrets returns every secret of the kinds given, or of any kind
+// if none is, which the server has handed to a client, in order.
+func (s *Server) IssuedSecrets(kinds ...SecretKind) []string {
 	s.mu.Lock()
 	defer s.mu.Unlock()
-	return slices.Clone(s.secrets)
+	var out []string
+	for _, secret := range s.secrets {
+		if len(kinds) == 0 || slices.Contains(kinds, secret.kind) {
+			out = append(out, secret.value)
+		}
+	}
+	return out
 }
 
 // Inject adds a fault. Faults are checked in the order they were added

backend/protondrive/protontest/upload.go

@@ -426,7 +426,7 @@ func (s *Server) handleRequestUpload(w http.ResponseWriter, r *http.Request, u *
 	resp := &blockUploadResponse{UploadLinks: []blockURLJSON{}, ThumbnailLinks: []struct{}{}, Code: api.CodeOK}
 	for _, b := range blocks {
 		rev.pending[b.index] = b
-		id, token := randomID(), s.newSecretLocked()
+		id, token := randomID(), s.newSecretLocked(StorageToken)
 		s.uploads[id] = &uploadURL{token: token, user: u, linkID: l.id, rev: rev, block: b}
 		bareURL := s.URL() + "/storage/upload/" + id
 		resp.UploadLinks = append(resp.UploadLinks, blockURLJSON{BareURL: bareURL, Token: token, URL: bareURL, Index: b.index})

backend/protondrive/security_test.go

@@ -20,11 +20,14 @@ import (
 	"github.com/rclone/rclone/backend/protondrive/protontest"
 	"github.com/rclone/rclone/fs"
 	"github.com/rclone/rclone/fs/accounting"
+	"github.com/rclone/rclone/fs/config"
+	"github.com/rclone/rclone/fs/config/configfile"
 	"github.com/rclone/rclone/fs/config/configmap"
 	"github.com/rclone/rclone/fs/config/obscure"
 	"github.com/rclone/rclone/fs/log"
 	"github.com/rclone/rclone/fs/object"
 	"github.com/rclone/rclone/fs/operations"
+	"github.com/rclone/rclone/fs/rc"
 	fssync "github.com/rclone/rclone/fs/sync"
 	"github.com/rclone/rclone/lib/random"
 	"github.com/stretchr/testify/assert"
@@ -244,6 +247,24 @@ func TestHostileServer(t *testing.T) {
 		require.NoError(t, err, "upload with signature checks off")
 	})
 
+	t.Run("updating a file with an attacker-signed passphrase", func(t *testing.T) {
+		ctx := context.Background()
+		remote, _, u := setupFakeRemote(t)
+		id := u.SeedFile(u.RootID(), "victim.bin", []byte("original"), time.Now())
+		u.Corrupt(id, "passphrase-signature")
+		f := newFakeFs(t, remote, "")
+		o, err := f.NewObject(ctx, "victim.bin")
+		require.NoError(t, err)
+		err = o.Update(ctx, bytes.NewReader([]byte("secret")), object.NewStaticObjectInfo("victim.bin", time.Now(), 6, true, nil, nil))
+		require.Error(t, err, "a new revision was encrypted to the attacker's key")
+		assert.Contains(t, err.Error(), "ignore_signature_errors")
+		for _, n := range u.Dump() {
+			if n.ID == id {
+				assert.Equal(t, "original", string(n.Content))
+			}
+		}
+	})
+
 	t.Run("names", func(t *testing.T) {
 		// Its own stats, so the sync sees only its own errors.
 		ctx := accounting.WithStatsGroup(context.Background(), "hostile names")
@@ -382,7 +403,7 @@ type secretRun struct {
 	log           string
 	passwords     [][]byte // the password and mailbox password
 	keyPassword   []byte
-	sessionTokens [][]byte // every access and refresh token saved
+	sessionTokens [][]byte // every access and refresh token issued
 	storageTokens [][]byte // every block upload and download token
 	nodeSecrets   [][]byte // node passphrases and content session keys
 }
@@ -433,20 +454,21 @@ func runSignedInSession(t *testing.T, dump fs.DumpFlags) secretRun {
 		passwords:   [][]byte{[]byte(password), []byte(mailboxPassword)},
 		keyPassword: []byte(u.KeyPassword()),
 	}
-	saved := map[string]bool{}
+	// Every token issued, including those never saved.
+	for _, token := range srv.IssuedSecrets(protontest.SessionToken) {
+		run.sessionTokens = append(run.sessionTokens, []byte(token))
+	}
 	for _, key := range []string{"client_access_token", "client_refresh_token"} {
-		for _, token := range m.values(key) {
+		saved := m.values(key)
+		require.NotEmpty(t, saved, key)
+		for _, token := range saved {
 			if token != "" {
-				saved[token] = true
-				run.sessionTokens = append(run.sessionTokens, []byte(token))
+				require.Contains(t, run.sessionTokens, []byte(token), "%s isn't one the server issued", key)
 			}
 		}
 	}
-	require.NotEmpty(t, run.sessionTokens)
-	for _, secret := range srv.IssuedSecrets() {
-		if !saved[secret] {
-			run.storageTokens = append(run.storageTokens, []byte(secret))
-		}
+	for _, token := range srv.IssuedSecrets(protontest.StorageToken) {
+		run.storageTokens = append(run.storageTokens, []byte(token))
 	}
 	require.NotEmpty(t, run.storageTokens)
 	for _, id := range []string{uploaded.(fs.IDer).ID(), stored, damaged} {
@@ -539,44 +561,50 @@ func TestConfigHoldsNoPasswordAfterBrowserSignIn(t *testing.T) {
 	assertNoSecrets(t, log, "key password", []byte(u.KeyPassword()))
 }
 
-// TestPasswordStoredObscured checks password sign-in keeps the password
-// obscured in the config, as rclone config stores it, and writes no
+// TestPasswordStoredObscured checks rclone config, given the password
+// in plain text, stores it obscured and signs in with it, writing no
 // password to the config itself.
 func TestPasswordStoredObscured(t *testing.T) {
 	ctx := context.Background()
 	const password, mailboxPassword = "stored-Hunter2-1a2b3c", "stored-mailbox-4d5e6f"
 	srv := protontest.New(t)
 	srv.AddTwoPasswordUser("user", password, mailboxPassword)
+	oldPath := config.GetConfigPath()
+	path := filepath.Join(t.TempDir(), "rclone.conf")
+	require.NoError(t, config.SetConfigPath(path))
+	t.Cleanup(func() { _ = config.SetConfigPath(oldPath) })
+	configfile.Install()
 
-	// rclone config obscures the options marked as passwords.
-	info, err := fs.Find("protondrive")
-	require.NoError(t, err)
-	for _, key := range []string{"password", "mailbox_password", "otp_secret_key"} {
-		opt := info.Options.Get(key)
-		require.NotNil(t, opt, key)
-		assert.True(t, opt.IsPassword, "%s is not obscured", key)
-	}
-
-	m := newLockedMap(configmap.Simple{
+	// As "rclone config create obscured protondrive ... --obscure".
+	opt := config.UpdateRemoteOpt{Obscure: true, NonInteractive: true}
+	out, err := config.CreateRemote(ctx, "obscured", "protondrive", rc.Params{
 		"endpoint":         srv.URL(),
 		"username":         "user",
-		"password":         obscure.MustObscure(password),
-		"mailbox_password": obscure.MustObscure(mailboxPassword),
-	})
-	_, err = protondrive.NewFs(ctx, "obscured", "", m)
-	require.NoError(t, err)
+		"password":         password,
+		"mailbox_password": mailboxPassword,
+	}, opt)
+	for range 5 {
+		require.NoError(t, err)
+		if out == nil || out.State == "" {
+			break
+		}
+		require.Equal(t, "choose", out.State, "unexpected question: %+v", out.Option)
+		out, err = config.UpdateRemote(ctx, "obscured", rc.Params{}, config.UpdateRemoteOpt{Continue: true, NonInteractive: true, State: out.State, Result: "false"})
+	}
+	require.True(t, out == nil || out.State == "", "config didn't finish")
+
 	for key, plain := range map[string]string{"password": password, "mailbox_password": mailboxPassword} {
-		stored, _ := m.Get(key)
+		stored, ok := config.FileGetValue("obscured", key)
+		require.True(t, ok, key)
 		assert.NotEqual(t, plain, stored, key)
 		revealed, err := obscure.Reveal(stored)
 		require.NoError(t, err, key)
 		assert.Equal(t, plain, revealed, key)
 	}
-	for key, values := range allValues(m) {
-		for _, v := range values {
-			assert.NotContains(t, v, password, "%s holds the password", key)
-			assert.NotContains(t, v, mailboxPassword, "%s holds the mailbox password", key)
-		}
-	}
-	assert.NotEmpty(t, m.values("client_uid"), "the session is saved")
+	uid, _ := config.FileGetValue("obscured", "client_uid")
+	assert.NotEmpty(t, uid, "not signed in")
+	file, err := os.ReadFile(path)
+	require.NoError(t, err)
+	assert.NotContains(t, string(file), password)
+	assert.NotContains(t, string(file), mailboxPassword)
 }

docs/plans/001-protondrive-tier1.md

@@ -577,8 +577,9 @@ library what must stay readable.
     revision's blocks) and `"foreign-block"` (the first block encrypted with another
     content key).
   - Secrets for log hygiene tests (Task 16):
-    `func (s *Server) IssuedSecrets() []string` (every access and refresh
-    token, fork selector and block upload or download token issued) and
+    `func (s *Server) IssuedSecrets(kinds ...SecretKind) []string` (every
+    secret issued of the kinds `SessionToken`, `ForkSelector`,
+    `StorageToken` given, or all) and
     `func (u *User) NodeSecrets(id string) (passphrase, sessionKey []byte)`.
   - Hostile folder keys (Task 16 fix round):
     `func (u *User) SeedForgedFolder(parentID, name string) (id string)`