filer: accept gcs credentials file paths in the guarded remote client builder (#11296)

* filer: accept gcs credentials file paths in the guarded remote client builder

checkGcsCredentials rejected all filesystem paths, so a gcs mount
configured with remote.configure -gcs.appCredentialsFile (which stores
a path in GcsGoogleApplicationCredentials) was rejected by
BuildGuardedRemoteStorageClient with "gcs credentials must be inline
JSON". This broke existing gcs mounts on the volume, filer, and s3
remote-mount read paths that use the guarded builder.

Read and validate the file content instead of rejecting the path,
mirroring what the gcs client itself does in MakeWithHTTPClient. A path
that does not exist or does not contain valid gcs credentials is still
rejected before any client is built. guardedRemoteClient now reads the
file to extract the token exchange URL for the SSRF deny-list, so the
rebinding-safe dialer still guards the token endpoint.

* filer: resolve gcs credential paths and avoid leaking file existence

loadGcsCredentialsContent passed the raw credentials string to os.ReadFile,
so a documented ~/path (as written by remote.configure
-gcs.appCredentialsFile=~/...) was rejected because os.ReadFile does not
expand ~. It also wrapped the os.ReadFile error, which includes the
file path, exposing file existence to a caller who planted a conf with
an arbitrary path.

Resolve the path with util.ResolvePath, matching the gcs client's own
behavior in MakeWithHTTPClient. Return a generic sentinel error on read
failure so the path is not reflected in the error message. The credential
type validation still runs on the file content, so a path that does not
contain valid gcs credentials is rejected before any client is built.
This commit is contained in:
Chris Lu
2026-09-13 14:43:45 -07:00
committed by GitHub
parent 5d8a463b3e
commit 92c379e5b4
2 changed files with 69 additions and 10 deletions
+34 -9
View File
@@ -2,10 +2,12 @@ package weed_server
import (
"context"
"errors"
"fmt"
"net"
"net/http"
"net/url"
"os"
"slices"
"strings"
"sync"
@@ -21,6 +23,7 @@ import (
"github.com/seaweedfs/seaweedfs/weed/security"
"github.com/seaweedfs/seaweedfs/weed/storage/needle"
"github.com/seaweedfs/seaweedfs/weed/storage/types"
"github.com/seaweedfs/seaweedfs/weed/util"
)
// lookupIPAddrFunc resolves a host to one or more IP addresses. It is a
@@ -313,10 +316,12 @@ func guardedRemoteClient(remoteConf *remote_pb.RemoteConf) (endpoint string, mak
// gcs reaches a fixed object host, but the token exchange goes wherever the
// supplied credentials say, so guard that endpoint instead.
if remoteConf.Type == "gcs" && remoteConf.GcsGoogleApplicationCredentials != "" {
if _, tokenURL, err := gcsremote.ParseInlineCredentials(remoteConf.GcsGoogleApplicationCredentials); err == nil {
return tokenURL, func(httpClient *http.Client) (remote_storage.RemoteStorageClient, error) {
return gcsremote.MakeWithHTTPClient(remoteConf, httpClient, gcsremote.StaticKeyCredentialTypes...)
}, true
if data, err := loadGcsCredentialsContent(remoteConf.GcsGoogleApplicationCredentials); err == nil {
if _, tokenURL, parseErr := gcsremote.ParseInlineCredentials(string(data)); parseErr == nil {
return tokenURL, func(httpClient *http.Client) (remote_storage.RemoteStorageClient, error) {
return gcsremote.MakeWithHTTPClient(remoteConf, httpClient, gcsremote.StaticKeyCredentialTypes...)
}, true
}
}
}
return "", nil, false
@@ -328,6 +333,27 @@ func gcsCredentialsArePath(creds string) bool {
return creds != "" && !strings.HasPrefix(creds, "{")
}
var errGcsCredentialsUnreadable = errors.New("gcs credentials file is not readable or does not contain valid credentials")
// loadGcsCredentialsContent returns the credential JSON for a gcs credentials
// value, reading from disk when it is a filesystem path (as written by
// remote.configure -gcs.appCredentialsFile). This mirrors what the gcs client
// itself does in MakeWithHTTPClient, so the guard validates the same content
// the client will eventually load.
func loadGcsCredentialsContent(creds string) ([]byte, error) {
if creds == "" {
return nil, nil
}
if strings.HasPrefix(creds, "{") {
return []byte(creds), nil
}
data, err := os.ReadFile(util.ResolvePath(creds))
if err != nil {
return nil, errGcsCredentialsUnreadable
}
return data, nil
}
// checkGcsCredentials rejects a caller-supplied gcs credentials value that
// would make the SDK read from somewhere other than the credentials themselves,
// so the request fails before any client is built.
@@ -335,12 +361,11 @@ func checkGcsCredentials(creds string) error {
if creds == "" {
return nil
}
// A filesystem path is read from disk by the SDK. Accept only inline JSON
// on the request; the server env var still supplies a path.
if gcsCredentialsArePath(creds) {
return fmt.Errorf("gcs credentials must be inline JSON")
data, err := loadGcsCredentialsContent(creds)
if err != nil {
return err
}
credType, _, parseErr := gcsremote.ParseInlineCredentials(creds)
credType, _, parseErr := gcsremote.ParseInlineCredentials(string(data))
if parseErr != nil {
return parseErr
}
+35 -1
View File
@@ -4,6 +4,8 @@ import (
"context"
"errors"
"net"
"os"
"path/filepath"
"strings"
"sync/atomic"
"testing"
@@ -666,8 +668,40 @@ func TestBuildGuardedRemoteStorageClient(t *testing.T) {
GcsGoogleApplicationCredentials: "/etc/hostname",
}
if _, err := BuildGuardedRemoteStorageClient(context.Background(), gcsPathCreds, false); err == nil {
t.Error("expected a gcs credentials path to be rejected")
t.Error("expected a non-credentials file path to be rejected")
} else if !strings.Contains(err.Error(), "reject remote credentials") {
t.Errorf("error = %v, want reject remote credentials", err)
} else if strings.Contains(err.Error(), "/etc/hostname") {
t.Errorf("error must not leak the file path: %v", err)
}
// A file path that points to valid GCS credentials should be accepted.
credsFile := filepath.Join(t.TempDir(), "service-account.json")
validCreds := `{"type":"service_account","token_uri":"https://oauth2.googleapis.com/token","client_email":"sa@example.iam.gserviceaccount.com","private_key":"-----BEGIN PRIVATE KEY-----\nMIIBVwIBADANBgkqhkiG9w0BAQEFAASCAUEwggE9AgEAAkEAxY\n-----END PRIVATE KEY-----\n","private_key_id":"key1"}`
if err := os.WriteFile(credsFile, []byte(validCreds), 0600); err != nil {
t.Fatalf("write creds file: %v", err)
}
gcsFileCreds := &remote_pb.RemoteConf{
Name: "good",
Type: "gcs",
GcsGoogleApplicationCredentials: credsFile,
}
if err := checkGcsCredentials(credsFile); err != nil {
t.Errorf("valid gcs credentials file should pass: %v", err)
}
if _, err := BuildGuardedRemoteStorageClient(context.Background(), gcsFileCreds, false); err != nil {
t.Errorf("valid gcs credentials file should build: %v", err)
}
// A nonexistent path must be rejected without leaking the path in the error.
gcsMissingCreds := &remote_pb.RemoteConf{
Name: "missing",
Type: "gcs",
GcsGoogleApplicationCredentials: filepath.Join(t.TempDir(), "does-not-exist.json"),
}
if _, err := BuildGuardedRemoteStorageClient(context.Background(), gcsMissingCreds, false); err == nil {
t.Error("expected a nonexistent credentials file to be rejected")
} else if strings.Contains(err.Error(), "does-not-exist") {
t.Errorf("error must not leak the file path: %v", err)
}
}