From 3572a5cc66447cf1ce1cc4114ea79de7d6ff9f5c Mon Sep 17 00:00:00 2001 From: Nathan Houle Date: Wed, 26 Aug 2026 14:11:25 -0700 Subject: [PATCH 1/4] build!: upgrade required go version to 1.24 We're on an ancient version of Go and don't test against modern versions. This bumps the floor for consumer Go versions to 1.24 and expands the matrix to test against 1.24.x through 1.27.x (current). --- .github/workflows/test.yml | 2 +- .github/workflows/verify-go-src.yml | 2 +- go.mod | 49 ++++++++++++++++++++++++++++- go.sum | 14 --------- 4 files changed, 50 insertions(+), 17 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 5db17ebc..3c9ac666 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -33,7 +33,7 @@ jobs: strategy: matrix: os: [ubuntu-latest, macOS-latest, windows-latest] - go_version: [1.19.x, 1.20.x] + go_version: [1.24.x, 1.25.x, 1.26.x, 1.27.x] runs-on: ${{ matrix.os }} steps: - name: Install Go ${{ matrix.go_version }} diff --git a/.github/workflows/verify-go-src.yml b/.github/workflows/verify-go-src.yml index 4f80a3c2..0b4f3867 100644 --- a/.github/workflows/verify-go-src.yml +++ b/.github/workflows/verify-go-src.yml @@ -13,7 +13,7 @@ jobs: strategy: matrix: os: [ubuntu-latest] - go_version: [1.20.x] + go_version: [1.27.x] runs-on: ${{ matrix.os }} steps: - name: Install Go ${{ matrix.go_version }} diff --git a/go.mod b/go.mod index e5c49bfe..87e7066f 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/netlify/open-api/v2 -go 1.13 +go 1.24 require ( github.com/Azure/go-autorest/autorest v0.10.1 @@ -15,5 +15,52 @@ require ( github.com/rsc/goversion v1.2.0 github.com/sirupsen/logrus v1.6.0 github.com/stretchr/testify v1.8.2 +) + +require ( + github.com/Azure/go-autorest/autorest/adal v0.8.2 // indirect + github.com/Azure/go-autorest/autorest/date v0.2.0 // indirect + github.com/Azure/go-autorest/logger v0.1.0 // indirect + github.com/Azure/go-autorest/tracing v0.5.0 // indirect + github.com/PuerkitoBio/purell v1.1.1 // indirect + github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578 // indirect + github.com/asaskevich/govalidator v0.0.0-20200907205600-7a23bdc65eef // indirect + github.com/davecgh/go-spew v1.1.1 // indirect + github.com/dgrijalva/jwt-go v3.2.0+incompatible // indirect + github.com/fsnotify/fsnotify v1.4.9 // indirect + github.com/go-openapi/analysis v0.19.16 // indirect + github.com/go-openapi/inflect v0.19.0 // indirect + github.com/go-openapi/jsonpointer v0.19.5 // indirect + github.com/go-openapi/jsonreference v0.19.5 // indirect + github.com/go-openapi/loads v0.20.0 // indirect + github.com/go-openapi/spec v0.20.0 // indirect + github.com/go-stack/stack v1.8.0 // indirect + github.com/gorilla/handlers v1.4.2 // indirect + github.com/hashicorp/hcl v1.0.0 // indirect + github.com/jessevdk/go-flags v1.4.0 // indirect + github.com/josharian/intern v1.0.0 // indirect + github.com/konsorten/go-windows-terminal-sequences v1.0.3 // indirect + github.com/kr/pretty v0.2.0 // indirect + github.com/kr/text v0.2.0 // indirect + github.com/magiconair/properties v1.8.1 // indirect + github.com/mailru/easyjson v0.7.6 // indirect + github.com/mitchellh/mapstructure v1.4.0 // indirect + github.com/pelletier/go-toml v1.8.0 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect + github.com/spf13/afero v1.2.2 // indirect + github.com/spf13/cast v1.3.1 // indirect + github.com/spf13/jwalterweatherman v1.1.0 // indirect + github.com/spf13/pflag v1.0.5 // indirect + github.com/spf13/viper v1.7.0 // indirect + github.com/subosito/gotenv v1.2.0 // indirect + github.com/toqueteos/webbrowser v1.2.0 // indirect + go.mongodb.org/mongo-driver v1.4.4 // indirect + golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4 // indirect golang.org/x/net v0.7.0 // indirect + golang.org/x/sys v0.5.0 // indirect + golang.org/x/text v0.7.0 // indirect + golang.org/x/tools v0.1.12 // indirect + gopkg.in/ini.v1 v1.57.0 // indirect + gopkg.in/yaml.v2 v2.4.0 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index ae500ec3..ff6d4247 100644 --- a/go.sum +++ b/go.sum @@ -420,7 +420,6 @@ github.com/xdg/scram v0.0.0-20180814205039-7eeb5667e42c/go.mod h1:lB8K/P019DLNhe github.com/xdg/stringprep v0.0.0-20180714160509-73f8eece6fdc/go.mod h1:Jhud4/sHMO4oL310DaZAKk9ZaJ08SJfe+sJh0HrGL1Y= github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2/go.mod h1:UETIi67q53MR2AWcXfiuqkDkRtnGDLqkBTpCHuJHxtU= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= -github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= go.etcd.io/bbolt v1.3.2/go.mod h1:IbVyRI1SCnLcuJnV2u8VeU0CEYM7e686BmAb1XKL+uU= go.mongodb.org/mongo-driver v1.0.3/go.mod h1:u7ryQJ+DOzQmeO7zB6MHyr8jkEQvC8vH7qLUO4lqsUM= go.mongodb.org/mongo-driver v1.1.1/go.mod h1:u7ryQJ+DOzQmeO7zB6MHyr8jkEQvC8vH7qLUO4lqsUM= @@ -448,7 +447,6 @@ golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8U golang.org/x/crypto v0.0.0-20191206172530-e9b2fee46413/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/crypto v0.0.0-20200604202706-70a84ac30bf9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= -golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20190306152737-a1d7652674e8/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20190510132918-efd6b22b2522/go.mod h1:ZjyILWgesfNpC6sMxTJOJm9Kp84zZh5NQWvqDGG3Qr8= @@ -494,8 +492,6 @@ golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLL golang.org/x/net v0.0.0-20200602114024-627f9648deb9/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A= golang.org/x/net v0.0.0-20201110031124-69a78807bb2b/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU= golang.org/x/net v0.0.0-20201202161906-c7110b5ffcbb/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU= -golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= -golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c= golang.org/x/net v0.7.0 h1:rJrUqqhjsgNp7KqAIc25s9pZnjU7TUcSY7HcVZjdn1g= golang.org/x/net v0.7.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= @@ -509,7 +505,6 @@ golang.org/x/sync v0.0.0-20190227155943-e225da77a7e6/go.mod h1:RxMgew5VJxzue5/jJ golang.org/x/sync v0.0.0-20190412183630-56d357773e84/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sys v0.0.0-20180823144017-11551d06cbcc/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20180905080454-ebe1bf3edb33/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= @@ -533,21 +528,13 @@ golang.org/x/sys v0.0.0-20191005200804-aed5e4c7ecf9/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200610111108-226ff32320da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.5.0 h1:MUK/U/4lj1t1oPg0HfuXDN/Z1wv31ZJ/YcPiGccS4DU= golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= -golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= -golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.4/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= -golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= golang.org/x/text v0.7.0 h1:4BRB4x83lYWy72KwLD/qYDuTu7q9PjSagHvijDw7cLo= golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= @@ -584,7 +571,6 @@ golang.org/x/tools v0.1.12 h1:VveCTK38A2rkS8ZqFY25HIDFscX5X9OoEhJd3quQmXU= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= -golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/api v0.4.0/go.mod h1:8k5glujaEP+g9n7WNsDg8QP6cUVNI86fCNMcbazEtwE= google.golang.org/api v0.7.0/go.mod h1:WtwebWUNSVBH/HAw79HIFXZNqEvBhG+Ra+ax0hx3E3M= From 80da05647dbe38f6c124eddc44d8101397c16cad Mon Sep 17 00:00:00 2001 From: Nathan Houle Date: Wed, 26 Aug 2026 15:55:39 -0700 Subject: [PATCH 2/4] refactor: modernize go:build directives --- go/porcelain/{deploy_unix.go => deploy_other.go} | 2 +- go/porcelain/deploy_windows.go | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) rename go/porcelain/{deploy_unix.go => deploy_other.go} (80%) diff --git a/go/porcelain/deploy_unix.go b/go/porcelain/deploy_other.go similarity index 80% rename from go/porcelain/deploy_unix.go rename to go/porcelain/deploy_other.go index 191808b1..c78a407c 100644 --- a/go/porcelain/deploy_unix.go +++ b/go/porcelain/deploy_other.go @@ -1,4 +1,4 @@ -// +build !windows +//go:build !windows package porcelain diff --git a/go/porcelain/deploy_windows.go b/go/porcelain/deploy_windows.go index 279c05be..02d280d3 100644 --- a/go/porcelain/deploy_windows.go +++ b/go/porcelain/deploy_windows.go @@ -1,3 +1,5 @@ +//go:build windows + package porcelain import ( From 3949f860c23d9ff74832dcfb2dfe48e078392e55 Mon Sep 17 00:00:00 2001 From: pieh Date: Mon, 31 Aug 2026 16:16:41 +0200 Subject: [PATCH 3/4] feat: confine DoDeploy filesystem access to the deploy directories Route the deploy's file reads through an os.Root opened for each top-level directory (publish dir, functions, edge functions, edge redirects, db migrations) instead of resolving absolute paths. Callers may pass pre-opened *os.Root handles via new optional DeployOptions fields; otherwise a directory is opened from its path without following a symlinked final component. File contents stream through the directory handle, and an upload whose file can no longer be opened fails instead of retrying. Requires Go 1.24 for os.Root. Co-authored-by: Nathan Houle --- go.mod | 2 +- go/porcelain/deploy.go | 444 +++++++++++++++++------- go/porcelain/deploy_containment_test.go | 116 +++++++ go/porcelain/deploy_other.go | 7 +- go/porcelain/deploy_test.go | 70 ++-- go/porcelain/deploy_windows.go | 11 +- go/porcelain/deploy_windows_test.go | 10 - 7 files changed, 489 insertions(+), 171 deletions(-) create mode 100644 go/porcelain/deploy_containment_test.go delete mode 100644 go/porcelain/deploy_windows_test.go diff --git a/go.mod b/go.mod index 87e7066f..d7f8b5d3 100644 --- a/go.mod +++ b/go.mod @@ -15,6 +15,7 @@ require ( github.com/rsc/goversion v1.2.0 github.com/sirupsen/logrus v1.6.0 github.com/stretchr/testify v1.8.2 + golang.org/x/sys v0.5.0 ) require ( @@ -57,7 +58,6 @@ require ( go.mongodb.org/mongo-driver v1.4.4 // indirect golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4 // indirect golang.org/x/net v0.7.0 // indirect - golang.org/x/sys v0.5.0 // indirect golang.org/x/text v0.7.0 // indirect golang.org/x/tools v0.1.12 // indirect gopkg.in/ini.v1 v1.57.0 // indirect diff --git a/go/porcelain/deploy.go b/go/porcelain/deploy.go index 038e488b..f9fb07de 100644 --- a/go/porcelain/deploy.go +++ b/go/porcelain/deploy.go @@ -13,6 +13,7 @@ import ( "fmt" "hash" "io" + "io/fs" "io/ioutil" "os" "path/filepath" @@ -91,6 +92,19 @@ type DeployOptions struct { LargeMediaEnabled bool Environment []*models.DeployEnvironmentVariable + // DirRoot and friends are optional pre-opened handles for the corresponding + // *Dir path fields. When set, all filesystem access for that directory goes + // through the handle; when nil, the path field is opened with [os.OpenRoot] + // once at the start of the deploy. Caller-provided handles must stay open for + // the duration of the deploy and are not closed by this package. The path + // fields should still be set: they are used for logging and to resolve paths + // read from manifest files. + DirRoot *os.Root + FunctionsDirRoot *os.Root + EdgeFunctionsDirRoot *os.Root + EdgeRedirectsDirRoot *os.Root + DbMigrationsDirRoot *os.Root + IsDraft bool SkipRetry bool @@ -139,6 +153,30 @@ type FileBundle struct { // pathReader is lazily opened from Path when Buffer is nil, so the deprecated Read/Seek/Close // methods keep working for external callers that treat a FileBundle as an io.ReadSeekCloser. pathReader *os.File + + // root is the directory handle the file lives in and rel its path within that + // handle; every read of the file's contents goes through root. + root *os.Root + rel string +} + +// open returns a reader for the bundle's contents, always through the root +// handle the bundle was created with. +func (f *FileBundle) open() (*os.File, error) { + if f.root == nil { + return nil, fmt.Errorf("file bundle %s has no root handle", f.Name) + } + return openRegularFileInRoot(f.root, f.rel) +} + +// legacyOpen backs the deprecated Read/Seek methods only: a FileBundle +// constructed by hand by an external caller has no root handle and keeps the +// historical direct open of its Path. +func (f *FileBundle) legacyOpen() (*os.File, error) { + if f.root == nil { + return os.Open(f.Path) + } + return f.open() } type FunctionMetadata struct { @@ -158,7 +196,7 @@ func (f *FileBundle) Read(p []byte) (n int, err error) { return f.Buffer.Read(p) } if f.pathReader == nil { - if f.pathReader, err = os.Open(f.Path); err != nil { + if f.pathReader, err = f.legacyOpen(); err != nil { return 0, err } } @@ -174,7 +212,7 @@ func (f *FileBundle) Seek(offset int64, whence int) (int64, error) { } if f.pathReader == nil { var err error - if f.pathReader, err = os.Open(f.Path); err != nil { + if f.pathReader, err = f.legacyOpen(); err != nil { return 0, err } } @@ -239,16 +277,113 @@ func (n *Netlify) DeploySite(ctx context.Context, options DeployOptions) (*model return n.DoDeploy(ctx, &options, nil) } +// dirHandle pairs an open directory handle with its configured path, which is +// used for messages, FileBundle.Path, and resolving absolute manifest paths. +type dirHandle struct { + root *os.Root + name string +} + +func (h dirHandle) valid() bool { + return h.root != nil +} + +// deployRoots holds the resolved directory handles for one deploy. Handles +// opened here (rather than provided by the caller) are recorded in owned and +// closed when the deploy returns. +type deployRoots struct { + dir, functions, edgeFunctions, edgeRedirects, dbMigrations dirHandle + + owned []*os.Root +} + +func (r *deployRoots) close() { + for _, root := range r.owned { + _ = root.Close() + } +} + +// resolveRoot returns the caller-provided handle when set, otherwise opens one +// for path; an empty path means the directory takes no part in the deploy. +func (r *deployRoots) resolveRoot(handle *os.Root, path string) (dirHandle, error) { + if handle != nil { + name := path + if name == "" { + name = handle.Name() + } + return dirHandle{root: handle, name: name}, nil + } + if path == "" { + return dirHandle{}, nil + } + if fi, err := os.Lstat(path); err != nil { + return dirHandle{}, err + } else if fi.Mode()&os.ModeSymlink != 0 { + return dirHandle{}, fmt.Errorf("%s is a symbolic link", path) + } + root, err := os.OpenRoot(path) + if err != nil { + return dirHandle{}, err + } + r.owned = append(r.owned, root) + return dirHandle{root: root, name: path}, nil +} + +func resolveDeployRoots(options *DeployOptions) (*deployRoots, error) { + roots := &deployRoots{} + + resolve := func(dst *dirHandle, handle *os.Root, path string) error { + h, err := roots.resolveRoot(handle, path) + if err != nil { + roots.close() + return err + } + *dst = h + return nil + } + + // Keep the historical error message for a path that is not a directory. + if options.DirRoot == nil { + f, err := os.Stat(options.Dir) + if err != nil { + return nil, err + } + if !f.IsDir() { + return nil, fmt.Errorf("%s is not a directory", options.Dir) + } + } + + if err := resolve(&roots.dir, options.DirRoot, options.Dir); err != nil { + return nil, err + } + if !roots.dir.valid() { + return nil, fmt.Errorf("no deploy directory provided") + } + if err := resolve(&roots.functions, options.FunctionsDirRoot, options.FunctionsDir); err != nil { + return nil, err + } + if err := resolve(&roots.edgeFunctions, options.EdgeFunctionsDirRoot, options.EdgeFunctionsDir); err != nil { + return nil, err + } + if err := resolve(&roots.edgeRedirects, options.EdgeRedirectsDirRoot, options.EdgeRedirectsDir); err != nil { + return nil, err + } + if err := resolve(&roots.dbMigrations, options.DbMigrationsDirRoot, options.DbMigrationsDir); err != nil { + return nil, err + } + + return roots, nil +} + // DoDeploy deploys the changes for a site given a directory in the filesystem. // It uploads the necessary files that changed between deploys. func (n *Netlify) DoDeploy(ctx context.Context, options *DeployOptions, deploy *models.Deploy) (*models.Deploy, error) { - f, err := os.Stat(options.Dir) + roots, err := resolveDeployRoots(options) if err != nil { return nil, err } - if !f.IsDir() { - return nil, fmt.Errorf("%s is not a directory", options.Dir) - } + // The upload phase re-reads every required file through these handles. + defer roots.close() if options.Observer != nil { if err := options.Observer.OnSetupWalk(); err != nil { @@ -257,10 +392,10 @@ func (n *Netlify) DoDeploy(ctx context.Context, options *DeployOptions, deploy * } largeMediaEnabled := options.LargeMediaEnabled - ignoreInstallDirs := options.Dir == options.BuildDir + ignoreInstallDirs := options.Dir != "" && options.Dir == options.BuildDir context.GetLogger(ctx).Infof("Getting files info with large media flag: %v", largeMediaEnabled) - files, err := walk(options.Dir, options.Observer, largeMediaEnabled, ignoreInstallDirs) + files, err := walk(roots.dir, options.Observer, largeMediaEnabled, ignoreInstallDirs) if err != nil { if options.Observer != nil { options.Observer.OnFailedWalk() @@ -273,8 +408,8 @@ func (n *Netlify) DoDeploy(ctx context.Context, options *DeployOptions, deploy * } } - if options.EdgeFunctionsDir != "" { - err = addInternalFilesToDeploy(options.EdgeFunctionsDir, edgeFunctionsInternalPath, files, options.Observer) + if roots.edgeFunctions.valid() { + err = addInternalFilesToDeploy(roots.edgeFunctions, edgeFunctionsInternalPath, files, options.Observer) if err != nil { if options.Observer != nil { options.Observer.OnFailedWalk() @@ -283,8 +418,8 @@ func (n *Netlify) DoDeploy(ctx context.Context, options *DeployOptions, deploy * } } - if options.EdgeRedirectsDir != "" { - err = addInternalFilesToDeploy(options.EdgeRedirectsDir, edgeRedirectsInternalPath, files, options.Observer) + if roots.edgeRedirects.valid() { + err = addInternalFilesToDeploy(roots.edgeRedirects, edgeRedirectsInternalPath, files, options.Observer) if err != nil { if options.Observer != nil { options.Observer.OnFailedWalk() @@ -293,8 +428,8 @@ func (n *Netlify) DoDeploy(ctx context.Context, options *DeployOptions, deploy * } } - if options.DbMigrationsDir != "" { - err = addInternalFilesToDeploy(options.DbMigrationsDir, dbMigrationsInternalPath, files, options.Observer) + if roots.dbMigrations.valid() { + err = addInternalFilesToDeploy(roots.dbMigrations, dbMigrationsInternalPath, files, options.Observer) if err != nil { if options.Observer != nil { options.Observer.OnFailedWalk() @@ -311,7 +446,7 @@ func (n *Netlify) DoDeploy(ctx context.Context, options *DeployOptions, deploy * functionsTmpDir := &lazyTempDir{} defer functionsTmpDir.remove() - functions, schedules, functionsConfig, err := bundle(ctx, options.FunctionsDir, functionsTmpDir, options.Observer) + functions, schedules, functionsConfig, err := bundle(ctx, roots.functions, functionsTmpDir, options.Observer) if err != nil { if options.Observer != nil { options.Observer.OnFailedWalk() @@ -322,7 +457,7 @@ func (n *Netlify) DoDeploy(ctx context.Context, options *DeployOptions, deploy * options.functionSchedules = schedules options.functionsConfig = functionsConfig - edgeFunctions, err := bundleEdgeFunctions(ctx, options.EdgeFunctionsDir, options.Observer) + edgeFunctions, err := bundleEdgeFunctions(ctx, roots.edgeFunctions, options.Observer) if err != nil { if options.Observer != nil { options.Observer.OnFailedWalk() @@ -602,58 +737,51 @@ func (n *Netlify) uploadFile(ctx context.Context, d *models.Deploy, f *FileBundl } sharedErr.mutex.Unlock() - var operationError error + // Opening the file cannot start succeeding on a retry, so fail permanently + // rather than backing off for the full retry window. + body, openErr := f.open() + if openErr != nil { + context.GetLogger(ctx).WithError(openErr).Errorf("Failed to open %v for upload", f.Name) + return backoff.Permanent(openErr) + } + defer func() { _ = body.Close() }() + var operationError error switch t { case fileUpload: - var body io.ReadCloser - body, operationError = os.Open(f.Path) - if operationError == nil { - defer body.Close() - params := operations.NewUploadDeployFileParams().WithDeployID(d.ID).WithPath(f.Name).WithFileBody(body) - if f.Size != nil { - params.WithSize(f.Size) - } - if timeout != 0 { - params.SetTimeout(timeout) - } - _, operationError = n.Operations.UploadDeployFile(params, authInfo) + params := operations.NewUploadDeployFileParams().WithDeployID(d.ID).WithPath(f.Name).WithFileBody(body) + if f.Size != nil { + params.WithSize(f.Size) } + if timeout != 0 { + params.SetTimeout(timeout) + } + _, operationError = n.Operations.UploadDeployFile(params, authInfo) case functionUpload: - var body io.ReadCloser - body, operationError = os.Open(f.Path) - if operationError == nil { - defer body.Close() - params := operations.NewUploadDeployFunctionParams().WithDeployID(d.ID).WithName(f.Name).WithFileBody(body).WithRuntime(&f.Runtime) - - if retryCount > 0 { - params = params.WithXNfRetryCount(&retryCount) - } + params := operations.NewUploadDeployFunctionParams().WithDeployID(d.ID).WithName(f.Name).WithFileBody(body).WithRuntime(&f.Runtime) - if f.FunctionMetadata != nil { - params = params.WithInvocationMode(&f.FunctionMetadata.InvocationMode) - params = params.WithTimeout(&f.FunctionMetadata.Timeout) - } + if retryCount > 0 { + params = params.WithXNfRetryCount(&retryCount) + } - if timeout != 0 { - params.SetRequestTimeout(timeout) - } - _, operationError = n.Operations.UploadDeployFunction(params, authInfo) + if f.FunctionMetadata != nil { + params = params.WithInvocationMode(&f.FunctionMetadata.InvocationMode) + params = params.WithTimeout(&f.FunctionMetadata.Timeout) + } + + if timeout != 0 { + params.SetRequestTimeout(timeout) } + _, operationError = n.Operations.UploadDeployFunction(params, authInfo) case edgeFunctionUpload: - var body io.ReadCloser - body, operationError = os.Open(f.Path) - if operationError == nil { - defer body.Close() - params := operations.NewUploadDeployEdgeFunctionParams().WithDeployID(d.ID).WithCodeSha(f.Sum).WithFileBody(body) - if retryCount > 0 { - params = params.WithXNfRetryCount(&retryCount) - } - if timeout != 0 { - params.SetTimeout(timeout) - } - _, operationError = n.Operations.UploadDeployEdgeFunction(params, authInfo) + params := operations.NewUploadDeployEdgeFunctionParams().WithDeployID(d.ID).WithCodeSha(f.Sum).WithFileBody(body) + if retryCount > 0 { + params = params.WithXNfRetryCount(&retryCount) + } + if timeout != 0 { + params.SetTimeout(timeout) } + _, operationError = n.Operations.UploadDeployEdgeFunction(params, authInfo) } if operationError != nil { @@ -697,24 +825,28 @@ func (n *Netlify) uploadFile(ctx context.Context, d *models.Deploy, f *FileBundl } } -func createFileBundle(rel, path string) (*FileBundle, error) { - return createFileBundleWithHasher(rel, path, sha1.New()) +func createFileBundle(rel string, dir dirHandle, relPath string) (*FileBundle, error) { + return createFileBundleWithHasher(rel, dir, relPath, sha1.New()) } -func createFunctionFileBundle(rel, path string) (*FileBundle, error) { - return createFileBundleWithHasher(rel, path, sha256.New()) +func createFunctionFileBundle(rel string, dir dirHandle, relPath string) (*FileBundle, error) { + return createFileBundleWithHasher(rel, dir, relPath, sha256.New()) } -func createFileBundleWithHasher(rel, path string, s hash.Hash) (*FileBundle, error) { - o, err := os.Open(path) +// createFileBundleWithHasher builds the bundle for the file at relPath inside +// dir; rel is the name it deploys as. +func createFileBundleWithHasher(rel string, dir dirHandle, relPath string, s hash.Hash) (*FileBundle, error) { + o, err := openRegularFileInRoot(dir.root, relPath) if err != nil { return nil, err } - defer o.Close() + defer func() { _ = o.Close() }() file := &FileBundle{ Name: rel, - Path: path, + Path: filepath.Join(dir.name, relPath), + root: dir.root, + rel: relPath, } if _, err := io.Copy(s, o); err != nil { @@ -726,32 +858,46 @@ func createFileBundleWithHasher(rel, path string, s hash.Hash) (*FileBundle, err return file, nil } -func walk(dir string, observer DeployObserver, useLargeMedia, ignoreInstallDirs bool) (*deployFiles, error) { +// openRegularFileInRoot opens relPath inside root for reading, rejecting +// anything but a regular file. O_NONBLOCK avoids blocking the open if relPath +// is a FIFO. +func openRegularFileInRoot(root *os.Root, relPath string) (*os.File, error) { + f, err := root.OpenFile(filepath.FromSlash(relPath), os.O_RDONLY|openNonblock, 0) + if err != nil { + return nil, err + } + fi, err := f.Stat() + if err != nil { + _ = f.Close() + return nil, err + } + if !fi.Mode().IsRegular() { + _ = f.Close() + return nil, fmt.Errorf("%s is not a regular file", relPath) + } + return f, nil +} + +func walk(dir dirHandle, observer DeployObserver, useLargeMedia, ignoreInstallDirs bool) (*deployFiles, error) { files := newDeployFiles() - err := filepath.Walk(dir, func(path string, info os.FileInfo, err error) error { + err := fs.WalkDir(dir.root.FS(), ".", func(rel string, d fs.DirEntry, err error) error { if err != nil { return err } - if !info.IsDir() && info.Mode().IsRegular() { - osRel, err := filepath.Rel(dir, path) - if err != nil { - return err - } - rel := forceSlashSeparators(osRel) - + if !d.IsDir() && d.Type().IsRegular() { if ignoreFile(rel, ignoreInstallDirs) { return nil } - file, err := createFileBundle(rel, path) + file, err := createFileBundle(rel, dir, rel) if err != nil { return err } if useLargeMedia { - o, err := os.Open(path) + o, err := openRegularFileInRoot(dir.root, rel) if err != nil { return err } @@ -786,20 +932,16 @@ func walk(dir string, observer DeployObserver, useLargeMedia, ignoreInstallDirs return files, err } -func addInternalFilesToDeploy(dir, internalPath string, files *deployFiles, observer DeployObserver) error { - return filepath.Walk(dir, func(path string, info os.FileInfo, err error) error { +func addInternalFilesToDeploy(dir dirHandle, internalPath string, files *deployFiles, observer DeployObserver) error { + return fs.WalkDir(dir.root.FS(), ".", func(osRel string, d fs.DirEntry, err error) error { if err != nil { return err } - if !info.IsDir() && info.Mode().IsRegular() { - osRel, err := filepath.Rel(dir, path) - if err != nil { - return err - } - rel := internalPath + forceSlashSeparators(osRel) + if !d.IsDir() && d.Type().IsRegular() { + rel := internalPath + osRel - file, err := createFileBundle(rel, path) + file, err := createFileBundle(rel, dir, osRel) if err != nil { return err } @@ -820,70 +962,83 @@ func addInternalFilesToDeploy(dir, internalPath string, files *deployFiles, obse type lazyTempDir struct { root string path string + handle *os.Root created bool } -func (l *lazyTempDir) get() (string, error) { +func (l *lazyTempDir) get() (string, *os.Root, error) { if !l.created { path, err := os.MkdirTemp(l.root, "netlify-deploy-functions-") if err != nil { - return "", err + return "", nil, err + } + handle, err := os.OpenRoot(path) + if err != nil { + os.RemoveAll(path) + return "", nil, err } - l.path, l.created = path, true + l.path, l.handle, l.created = path, handle, true } - return l.path, nil + return l.path, l.handle, nil } func (l *lazyTempDir) remove() { if l.created { + _ = l.handle.Close() os.RemoveAll(l.path) } } -func bundle(ctx context.Context, functionDir string, tmpDir *lazyTempDir, observer DeployObserver) (*deployFiles, []*models.FunctionSchedule, map[string]models.FunctionConfig, error) { - if functionDir == "" { +func bundle(ctx context.Context, functionsDir dirHandle, tmpDir *lazyTempDir, observer DeployObserver) (*deployFiles, []*models.FunctionSchedule, map[string]models.FunctionConfig, error) { + if !functionsDir.valid() { return nil, nil, nil, nil } - manifestFile, err := os.Open(filepath.Join(functionDir, "manifest.json")) + manifestFile, err := openRegularFileInRoot(functionsDir.root, "manifest.json") // If a `manifest.json` file is found, we extract the functions and their // metadata from it. if err == nil { defer manifestFile.Close() - return bundleFromManifest(ctx, manifestFile, tmpDir, observer) + return bundleFromManifest(ctx, functionsDir, manifestFile, tmpDir, observer) } functions := newDeployFiles() - info, err := ioutil.ReadDir(functionDir) + info, err := fs.ReadDir(functionsDir.root.FS(), ".") if err != nil { return nil, nil, nil, err } - for _, i := range info { - filePath := filepath.Join(functionDir, i.Name()) + for _, entry := range info { + i, err := entry.Info() + if err != nil { + return nil, nil, nil, err + } + + // filePath is only used for warnings and the go-binary classification. + filePath := filepath.Join(functionsDir.name, i.Name()) switch { case zipFile(i): - runtime, err := readZipRuntime(filePath) + runtime, err := readZipRuntime(functionsDir.root, i.Name()) if err != nil { return nil, nil, nil, err } - file, err := newFunctionFile(filePath, i, runtime, nil, tmpDir, observer) + file, err := newFunctionFile(functionsDir, i.Name(), i, runtime, nil, tmpDir, observer) if err != nil { return nil, nil, nil, err } functions.Add(file.Name, file) case jsFile(i): - file, err := newFunctionFile(filePath, i, jsRuntime, nil, tmpDir, observer) + file, err := newFunctionFile(functionsDir, i.Name(), i, jsRuntime, nil, tmpDir, observer) if err != nil { return nil, nil, nil, err } functions.Add(file.Name, file) case goFile(filePath, i, observer): - file, err := newFunctionFile(filePath, i, amazonLinux2, nil, tmpDir, observer) + file, err := newFunctionFile(functionsDir, i.Name(), i, amazonLinux2, nil, tmpDir, observer) if err != nil { return nil, nil, nil, err } @@ -898,7 +1053,7 @@ func bundle(ctx context.Context, functionDir string, tmpDir *lazyTempDir, observ return functions, nil, nil, nil } -func bundleFromManifest(ctx context.Context, manifestFile *os.File, tmpDir *lazyTempDir, observer DeployObserver) (*deployFiles, []*models.FunctionSchedule, map[string]models.FunctionConfig, error) { +func bundleFromManifest(ctx context.Context, functionsDir dirHandle, manifestFile *os.File, tmpDir *lazyTempDir, observer DeployObserver) (*deployFiles, []*models.FunctionSchedule, map[string]models.FunctionConfig, error) { manifestBytes, err := ioutil.ReadAll(manifestFile) if err != nil { return nil, nil, nil, err @@ -919,7 +1074,14 @@ func bundleFromManifest(ctx context.Context, manifestFile *os.File, tmpDir *lazy functionsConfig := make(map[string]models.FunctionConfig) for _, function := range manifest.Functions { - fileInfo, err := os.Stat(function.Path) + // The manifest is untrusted input: the paths it names must resolve + // inside the functions directory. + relPath, err := manifestFunctionRel(functionsDir.name, function.Path) + if err != nil { + return nil, nil, nil, err + } + + fileInfo, err := functionsDir.root.Stat(relPath) if err != nil { return nil, nil, nil, fmt.Errorf("manifest file specifies a function path that cannot be found: %s", function.Path) } @@ -935,7 +1097,7 @@ func bundleFromManifest(ctx context.Context, manifestFile *os.File, tmpDir *lazy InvocationMode: function.InvocationMode, Timeout: function.Timeout, } - file, err := newFunctionFile(function.Path, fileInfo, runtime, &meta, tmpDir, observer) + file, err := newFunctionFile(functionsDir, relPath, fileInfo, runtime, &meta, tmpDir, observer) if err != nil { return nil, nil, nil, err } @@ -1008,12 +1170,43 @@ func bundleFromManifest(ctx context.Context, manifestFile *os.File, tmpDir *lazy return functions, schedules, functionsConfig, nil } -func readZipRuntime(filePath string) (string, error) { - zf, err := zip.OpenReader(filePath) +// manifestFunctionRel converts a manifest function path into a path relative to +// the functions directory, rejecting anything that points outside it. +func manifestFunctionRel(rootName, path string) (string, error) { + rel := path + if filepath.IsAbs(path) { + absRoot, err := filepath.Abs(rootName) + if err != nil { + return "", err + } + rel, err = filepath.Rel(absRoot, path) + if err != nil { + return "", fmt.Errorf("manifest file specifies a function path outside the functions directory: %s", path) + } + } + rel = filepath.Clean(rel) + if rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return "", fmt.Errorf("manifest file specifies a function path outside the functions directory: %s", path) + } + return rel, nil +} + +func readZipRuntime(root *os.Root, relPath string) (string, error) { + f, err := openRegularFileInRoot(root, relPath) + if err != nil { + return "", err + } + defer func() { _ = f.Close() }() + + info, err := f.Stat() + if err != nil { + return "", err + } + + zf, err := zip.NewReader(f, info.Size()) if err != nil { return "", err } - defer zf.Close() for _, file := range zf.File { if file.Name == "netlify-toolchain" { @@ -1023,7 +1216,7 @@ func readZipRuntime(filePath string) (string, error) { // This preserves the current behavior in this library. return jsRuntime, nil } - defer fc.Close() + defer func() { _ = fc.Close() }() var tc toolchainSpec if err := json.NewDecoder(fc).Decode(&tc); err != nil { @@ -1038,15 +1231,15 @@ func readZipRuntime(filePath string) (string, error) { return jsRuntime, nil } -func newFunctionFile(filePath string, i os.FileInfo, runtime string, metadata *FunctionMetadata, tmpDir *lazyTempDir, observer DeployObserver) (*FileBundle, error) { +func newFunctionFile(dir dirHandle, relPath string, i os.FileInfo, runtime string, metadata *FunctionMetadata, tmpDir *lazyTempDir, observer DeployObserver) (*FileBundle, error) { var file *FileBundle var err error if zipFile(i) || tarFile(i) { name := strings.TrimSuffix(i.Name(), filepath.Ext(i.Name())) - file, err = createFunctionFileBundle(name, filePath) + file, err = createFunctionFileBundle(name, dir, relPath) } else { - file, err = zipFunctionFile(filePath, i, runtime, tmpDir) + file, err = zipFunctionFile(dir, relPath, i, runtime, tmpDir) } if err != nil { return nil, err @@ -1064,19 +1257,19 @@ func newFunctionFile(filePath string, i os.FileInfo, runtime string, metadata *F return file, nil } -func zipFunctionFile(filePath string, i os.FileInfo, runtime string, tmpDir *lazyTempDir) (*FileBundle, error) { - src, err := os.Open(filePath) +func zipFunctionFile(dir dirHandle, relPath string, i os.FileInfo, runtime string, tmpDir *lazyTempDir) (*FileBundle, error) { + src, err := openRegularFileInRoot(dir.root, relPath) if err != nil { return nil, err } - defer src.Close() + defer func() { _ = src.Close() }() - dir, err := tmpDir.get() + tmpPath, tmpRoot, err := tmpDir.get() if err != nil { return nil, err } - tmp, err := os.CreateTemp(dir, "function-*.zip") + tmp, err := os.CreateTemp(tmpPath, "function-*.zip") if err != nil { return nil, err } @@ -1110,6 +1303,8 @@ func zipFunctionFile(filePath string, i os.FileInfo, runtime string, tmpDir *laz Name: strings.TrimSuffix(i.Name(), filepath.Ext(i.Name())), Sum: hex.EncodeToString(s.Sum(nil)), Path: tmpName, + root: tmpRoot, + rel: filepath.Base(tmpName), }, nil } @@ -1117,18 +1312,23 @@ func zipFunctionFile(filePath string, i os.FileInfo, runtime string, tmpDir *laz // lists into an uploadable FileBundle. The deploy declares these as its edge_functions map // ({format => code_sha}); the server replies with the subset (required_edge_functions) not already // stored, and only those are streamed up. A missing manifest means no edge functions to upload. -func bundleEdgeFunctions(ctx context.Context, edgeFunctionsDir string, observer DeployObserver) (*deployFiles, error) { - if edgeFunctionsDir == "" { +func bundleEdgeFunctions(ctx context.Context, edgeFunctionsDir dirHandle, observer DeployObserver) (*deployFiles, error) { + if !edgeFunctionsDir.valid() { return nil, nil } - manifestBytes, err := os.ReadFile(filepath.Join(edgeFunctionsDir, "manifest.json")) + manifestFile, err := openRegularFileInRoot(edgeFunctionsDir.root, "manifest.json") if os.IsNotExist(err) { return nil, nil } if err != nil { return nil, err } + manifestBytes, err := io.ReadAll(manifestFile) + manifestFile.Close() + if err != nil { + return nil, err + } context.GetLogger(ctx).Debug("Found edge functions manifest file") @@ -1159,14 +1359,12 @@ func bundleEdgeFunctions(ctx context.Context, edgeFunctionsDir string, observer return files, nil } -func newEdgeFunctionFile(edgeFunctionsDir string, bundle edgeFunctionsManifestBundle) (*FileBundle, error) { - path := filepath.Join(edgeFunctionsDir, bundle.Asset) - +func newEdgeFunctionFile(edgeFunctionsDir dirHandle, bundle edgeFunctionsManifestBundle) (*FileBundle, error) { // code_sha is the dedup key in the deployer<->functions-origin contract, so we compute it from the // bundle's bytes rather than trusting the edge-bundler's asset filename (which currently also happens // to be the sha256, but that's a bundler implementation detail). createFileBundleWithHasher streams // the bytes through the hasher, so the bundle is never held in memory. - file, err := createFileBundleWithHasher(bundle.Format, path, sha256.New()) + file, err := createFileBundleWithHasher(bundle.Format, edgeFunctionsDir, bundle.Asset, sha256.New()) if err != nil { return nil, fmt.Errorf("edge functions manifest specifies a bundle that cannot be read: %s: %w", bundle.Asset, err) } diff --git a/go/porcelain/deploy_containment_test.go b/go/porcelain/deploy_containment_test.go new file mode 100644 index 00000000..2f302a33 --- /dev/null +++ b/go/porcelain/deploy_containment_test.go @@ -0,0 +1,116 @@ +package porcelain + +import ( + gocontext "context" + "encoding/json" + "fmt" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// The deploy walk and the function/edge bundlers read every file through an +// os.Root over the directory they were given, so nothing outside that directory +// is ever hashed or uploaded. These tests assert that contract directly. + +// manifestFunctionRel is the guard that keeps a customer-controlled functions +// manifest from naming a path outside the functions directory. +func TestManifestFunctionRel(t *testing.T) { + root := filepath.Join("srv", "functions") + + cases := []struct { + name string + path string + want string + wantErr bool + }{ + {name: "plain file", path: "fn.zip", want: "fn.zip"}, + {name: "nested file", path: filepath.Join("sub", "fn.zip"), want: filepath.Join("sub", "fn.zip")}, + {name: "cleaned but in-root", path: filepath.Join("sub", "..", "fn.zip"), want: "fn.zip"}, + {name: "absolute in-root", path: filepath.Join(mustAbs(t, root), "fn.zip"), want: "fn.zip"}, + {name: "parent escape", path: filepath.Join("..", "escape"), wantErr: true}, + {name: "deep escape", path: filepath.Join("sub", "..", "..", "escape"), wantErr: true}, + {name: "bare parent", path: "..", wantErr: true}, + {name: "absolute escape", path: filepath.Join(mustAbs(t, "elsewhere"), "escape"), wantErr: true}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got, err := manifestFunctionRel(root, tc.path) + if tc.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tc.want, got) + }) + } +} + +func mustAbs(t *testing.T, p string) string { + t.Helper() + abs, err := filepath.Abs(p) + require.NoError(t, err) + return abs +} + +// bundle rejects a functions manifest whose path resolves outside the functions +// directory, and never reads the out-of-tree file it names. +func TestBundle_RejectsManifestPathEscapingRoot(t *testing.T) { + root := t.TempDir() + functionsDir := filepath.Join(root, "functions") + require.NoError(t, os.Mkdir(functionsDir, 0o755)) + + // A file outside the functions directory that the manifest tries to reach. + outside := filepath.Join(root, "secret.zip") + require.NoError(t, os.WriteFile(outside, []byte("out-of-tree"), 0o644)) + + manifest := map[string]any{ + "version": 1, + "functions": []map[string]any{ + {"name": "escape", "path": filepath.Join("..", "secret.zip"), "runtime": "go"}, + }, + } + body, err := json.Marshal(manifest) + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(functionsDir, "manifest.json"), body, 0o644)) + + _, _, _, err = bundle(gocontext.Background(), testDir(t, functionsDir), newTestTempDir(t), mockObserver{}) + require.Error(t, err) + assert.Contains(t, err.Error(), "outside the functions directory") +} + +// A symlink in the deploy directory is not followed: the walk records regular +// files only, so a link pointing outside the tree contributes nothing. +func TestWalk_DoesNotFollowSymlinkOutOfRoot(t *testing.T) { + root := t.TempDir() + deployDir := filepath.Join(root, "publish") + require.NoError(t, os.Mkdir(deployDir, 0o755)) + + require.NoError(t, os.WriteFile(filepath.Join(deployDir, "index.html"), []byte("ok"), 0o644)) + + outside := filepath.Join(root, "secret") + require.NoError(t, os.WriteFile(outside, []byte("out-of-tree"), 0o644)) + if err := os.Symlink(outside, filepath.Join(deployDir, "leak")); err != nil { + t.Skipf("symlinks unavailable on this platform: %v", err) + } + + files, err := walk(testDir(t, deployDir), mockObserver{}, false, false) + require.NoError(t, err) + + assert.NotNil(t, files.Files["index.html"], "regular file should be deployed") + assert.Nil(t, files.Files["leak"], "symlink should not be deployed") + for name := range files.Files { + assert.NotEqual(t, "out-of-tree", contentOf(t, deployDir, name)) + } +} + +func contentOf(t *testing.T, dir, name string) string { + t.Helper() + b, err := os.ReadFile(filepath.Join(dir, filepath.FromSlash(name))) + require.NoError(t, err) + return fmt.Sprintf("%s", b) +} diff --git a/go/porcelain/deploy_other.go b/go/porcelain/deploy_other.go index c78a407c..8959575d 100644 --- a/go/porcelain/deploy_other.go +++ b/go/porcelain/deploy_other.go @@ -2,6 +2,7 @@ package porcelain -func forceSlashSeparators(name string) string { - return name -} +import "syscall" + +// openNonblock keeps a FIFO from hanging a read-open; no effect on regular files. +const openNonblock = syscall.O_NONBLOCK diff --git a/go/porcelain/deploy_test.go b/go/porcelain/deploy_test.go index 7887cbe9..0f878e2b 100644 --- a/go/porcelain/deploy_test.go +++ b/go/porcelain/deploy_test.go @@ -30,6 +30,26 @@ import ( "github.com/stretchr/testify/require" ) +// testDir opens a dirHandle over dir for the lifetime of the test. +func testDir(t *testing.T, dir string) dirHandle { + t.Helper() + root, err := os.OpenRoot(dir) + require.NoError(t, err) + t.Cleanup(func() { _ = root.Close() }) + return dirHandle{root: root, name: dir} +} + +// newTestTempDir returns a lazyTempDir whose handle and directory are cleaned up +// when the test ends. Production pairs every lazyTempDir with a deferred remove; +// tests must too, or the still-open directory handle blocks TempDir cleanup on +// Windows. +func newTestTempDir(t *testing.T) *lazyTempDir { + t.Helper() + td := &lazyTempDir{root: t.TempDir()} + t.Cleanup(td.remove) + return td +} + func TestGetLFSSha(t *testing.T) { t.Run("test with not a pointer file", func(t *testing.T) { file := strings.NewReader("Not a pointer file") @@ -258,13 +278,13 @@ func TestWalk_IgnoreNodeModulesInRoot(t *testing.T) { err = ioutil.WriteFile(filepath.Join(dir, "more", "node_modules", "inner-package"), []byte{}, 0644) require.Nil(t, err) - files, err := walk(dir, mockObserver{}, false, false) + files, err := walk(testDir(t, dir), mockObserver{}, false, false) require.Nil(t, err) assert.NotNil(t, files.Files["node_modules/root-package"]) assert.NotNil(t, files.Files["more/node_modules/inner-package"]) // When deploy directory == build directory, ignore node_modules in deploy directory root. - files, err = walk(dir, mockObserver{}, false, true) + files, err = walk(testDir(t, dir), mockObserver{}, false, true) require.Nil(t, err) assert.Nil(t, files.Files["node_modules/root-package"]) assert.NotNil(t, files.Files["more/node_modules/inner-package"]) @@ -286,7 +306,7 @@ func TestWalk_EdgeFunctions(t *testing.T) { err = ioutil.WriteFile(filepath.Join(edgeFunctionsDir, "123456789.js"), []byte{}, 0644) require.Nil(t, err) - err = addInternalFilesToDeploy(edgeFunctionsDir, edgeFunctionsInternalPath, files, mockObserver{}) + err = addInternalFilesToDeploy(testDir(t, edgeFunctionsDir), edgeFunctionsInternalPath, files, mockObserver{}) require.Nil(t, err) assert.NotNil(t, files.Files[".netlify/internal/edge-functions/manifest.json"]) @@ -309,7 +329,7 @@ func TestWalk_PublishedFilesAndEdgeFunctions(t *testing.T) { err = ioutil.WriteFile(filepath.Join(edgeFunctionsDir, "123456789.js"), []byte{}, 0644) require.Nil(t, err) - err = addInternalFilesToDeploy(edgeFunctionsDir, edgeFunctionsInternalPath, files, mockObserver{}) + err = addInternalFilesToDeploy(testDir(t, edgeFunctionsDir), edgeFunctionsInternalPath, files, mockObserver{}) require.Nil(t, err) assert.NotNil(t, files.Files["assets/styles.css"]) @@ -332,7 +352,7 @@ func TestWalk_PublishedFilesAndEdgeRedirects(t *testing.T) { err = ioutil.WriteFile(filepath.Join(edgeRedirectsDir, "redirects.json"), []byte{}, 0644) require.Nil(t, err) - err = addInternalFilesToDeploy(edgeRedirectsDir, edgeRedirectsInternalPath, files, mockObserver{}) + err = addInternalFilesToDeploy(testDir(t, edgeRedirectsDir), edgeRedirectsInternalPath, files, mockObserver{}) require.Nil(t, err) assert.NotNil(t, files.Files["assets/styles.css"]) @@ -357,7 +377,7 @@ func TestWalk_DbMigrations(t *testing.T) { err = ioutil.WriteFile(filepath.Join(migrationDir, "migration.sql"), []byte("CREATE TABLE users (id INT);"), 0644) require.Nil(t, err) - err = addInternalFilesToDeploy(dbMigrationsDir, dbMigrationsInternalPath, files, mockObserver{}) + err = addInternalFilesToDeploy(testDir(t, dbMigrationsDir), dbMigrationsInternalPath, files, mockObserver{}) require.Nil(t, err) assert.NotNil(t, files.Files[".netlify/internal/db/migrations/1700000000_create-users/migration.sql"]) @@ -380,7 +400,7 @@ func TestWalk_PublishedFilesAndDbMigrations(t *testing.T) { err = ioutil.WriteFile(filepath.Join(migrationDir, "migration.sql"), []byte("CREATE TABLE users (id INT);"), 0644) require.Nil(t, err) - err = addInternalFilesToDeploy(dbMigrationsDir, dbMigrationsInternalPath, files, mockObserver{}) + err = addInternalFilesToDeploy(testDir(t, dbMigrationsDir), dbMigrationsInternalPath, files, mockObserver{}) require.Nil(t, err) assert.NotNil(t, files.Files["assets/styles.css"]) @@ -401,7 +421,7 @@ func setupPublishedAssets(t *testing.T) *deployFiles { err = ioutil.WriteFile(filepath.Join(publishDir, "index.html"), []byte{}, 0644) require.Nil(t, err) - files, err := walk(publishDir, mockObserver{}, false, false) + files, err := walk(testDir(t, publishDir), mockObserver{}, false, false) require.Nil(t, err) return files @@ -429,7 +449,7 @@ func TestUploadFiles_Cancelation(t *testing.T) { require.NoError(t, ioutil.WriteFile(filepath.Join(dir, "foo.html"), []byte("Hello"), 0644)) require.NoError(t, ioutil.WriteFile(filepath.Join(dir, "bar.html"), []byte("World"), 0644)) - files, err := walk(dir, nil, false, false) + files, err := walk(testDir(t, dir), nil, false, false) require.NoError(t, err) d := &models.Deploy{} for _, bundle := range files.Files { @@ -474,7 +494,7 @@ func TestUploadFiles_CancelationWaitsForInFlightUploads(t *testing.T) { require.NoError(t, os.WriteFile(filepath.Join(dir, "foo.html"), []byte("Hello"), 0644)) require.NoError(t, os.WriteFile(filepath.Join(dir, "bar.html"), []byte("World"), 0644)) - files, err := walk(dir, nil, false, false) + files, err := walk(testDir(t, dir), nil, false, false) require.NoError(t, err) d := &models.Deploy{} for _, bundle := range files.Files { @@ -535,7 +555,7 @@ func TestUploadFiles_Errors(t *testing.T) { defer os.RemoveAll(dir) require.NoError(t, ioutil.WriteFile(filepath.Join(dir, "foo.html"), []byte("Hello"), 0644)) - files, err := walk(dir, nil, false, false) + files, err := walk(testDir(t, dir), nil, false, false) require.NoError(t, err) d := &models.Deploy{} for _, bundle := range files.Files { @@ -573,7 +593,7 @@ func TestUploadFiles422Error_SkipsRetry(t *testing.T) { defer os.RemoveAll(dir) require.NoError(t, ioutil.WriteFile(filepath.Join(dir, "foo.html"), []byte("Hello"), 0644)) - files, err := walk(dir, nil, false, false) + files, err := walk(testDir(t, dir), nil, false, false) require.NoError(t, err) d := &models.Deploy{} for _, bundle := range files.Files { @@ -614,7 +634,7 @@ func TestUploadFunctions422Error_SkipsRetry(t *testing.T) { defer os.RemoveAll(dir) require.NoError(t, ioutil.WriteFile(filepath.Join(functionsPath, "foo.js"), []byte("module.exports = () => {}"), 0644)) - files, _, _, err := bundle(ctx, functionsPath, &lazyTempDir{root: t.TempDir()}, mockObserver{}) + files, _, _, err := bundle(ctx, testDir(t, functionsPath), newTestTempDir(t), mockObserver{}) require.NoError(t, err) d := &models.Deploy{} for _, bundle := range files.Files { @@ -654,7 +674,7 @@ func TestUploadFiles400Error_NoSkipRetry(t *testing.T) { defer os.RemoveAll(dir) require.NoError(t, ioutil.WriteFile(filepath.Join(dir, "foo.html"), []byte("Hello"), 0644)) - files, err := walk(dir, nil, false, false) + files, err := walk(testDir(t, dir), nil, false, false) require.NoError(t, err) d := &models.Deploy{} for _, bundle := range files.Files { @@ -695,7 +715,7 @@ func TestUploadFiles_SkipEqualFiles(t *testing.T) { require.NoError(t, ioutil.WriteFile(filepath.Join(dir, "a.html"), fileBody, 0644)) require.NoError(t, ioutil.WriteFile(filepath.Join(dir, "b.html"), fileBody, 0644)) - files, err := walk(dir, nil, false, false) + files, err := walk(testDir(t, dir), nil, false, false) require.NoError(t, err) // Create some fake function bundles to deploy @@ -713,7 +733,7 @@ func TestUploadFiles_SkipEqualFiles(t *testing.T) { require.NoError(t, ioutil.WriteFile(filepath.Join(functionsDir, "a.zip"), bundleBody, 0644)) require.NoError(t, ioutil.WriteFile(filepath.Join(functionsDir, "b.zip"), bundleBody, 0644)) - functions, _, _, err := bundle(ctx, functionsDir, &lazyTempDir{root: t.TempDir()}, mockObserver{}) + functions, _, _, err := bundle(ctx, testDir(t, functionsDir), newTestTempDir(t), mockObserver{}) require.NoError(t, err) d := &models.Deploy{} @@ -775,7 +795,7 @@ func TestUploadFunctions_RetryCountHeader(t *testing.T) { defer os.RemoveAll(dir) require.NoError(t, ioutil.WriteFile(filepath.Join(functionsPath, "foo.js"), []byte("module.exports = () => {}"), 0644)) - files, _, _, err := bundle(ctx, functionsPath, &lazyTempDir{root: t.TempDir()}, mockObserver{}) + files, _, _, err := bundle(ctx, testDir(t, functionsPath), newTestTempDir(t), mockObserver{}) require.NoError(t, err) d := &models.Deploy{} for _, bundle := range files.Files { @@ -800,7 +820,7 @@ func TestBundleEdgeFunctions(t *testing.T) { }` require.NoError(t, os.WriteFile(filepath.Join(dir, "manifest.json"), []byte(manifest), 0644)) - files, err := bundleEdgeFunctions(gocontext.Background(), dir, mockObserver{}) + files, err := bundleEdgeFunctions(gocontext.Background(), testDir(t, dir), mockObserver{}) require.NoError(t, err) // The declared edge_functions map keys each bundle by its format, with the code_sha computed by the @@ -819,12 +839,12 @@ func TestBundleEdgeFunctions(t *testing.T) { func TestBundleEdgeFunctions_NoManifest(t *testing.T) { // No edge functions dir configured. - files, err := bundleEdgeFunctions(gocontext.Background(), "", mockObserver{}) + files, err := bundleEdgeFunctions(gocontext.Background(), dirHandle{}, mockObserver{}) require.NoError(t, err) require.Nil(t, files) // A dir without a manifest.json yields no edge functions rather than an error. - files, err = bundleEdgeFunctions(gocontext.Background(), t.TempDir(), mockObserver{}) + files, err = bundleEdgeFunctions(gocontext.Background(), testDir(t, t.TempDir()), mockObserver{}) require.NoError(t, err) require.Nil(t, files) } @@ -855,7 +875,7 @@ func TestUploadEdgeFunctions(t *testing.T) { manifest := `{ "bundles": [ { "asset": "edgecodesha.tar.gz", "format": "tar" } ] }` require.NoError(t, os.WriteFile(filepath.Join(dir, "manifest.json"), []byte(manifest), 0644)) - files, err := bundleEdgeFunctions(gocontext.Background(), dir, mockObserver{}) + files, err := bundleEdgeFunctions(gocontext.Background(), testDir(t, dir), mockObserver{}) require.NoError(t, err) codeSha := files.Sums["tar"] @@ -868,7 +888,7 @@ func TestUploadEdgeFunctions(t *testing.T) { } func TestBundle(t *testing.T) { - functions, schedules, functionsConfig, err := bundle(gocontext.Background(), "../internal/data", &lazyTempDir{root: t.TempDir()}, mockObserver{}) + functions, schedules, functionsConfig, err := bundle(gocontext.Background(), testDir(t, "../internal/data"), newTestTempDir(t), mockObserver{}) assert.Nil(t, err) assert.Equal(t, 5, len(functions.Files)) @@ -943,7 +963,7 @@ func TestBundleWithManifest(t *testing.T) { defer os.Remove(manifestPath) assert.Nil(t, err) - functions, schedules, functionsConfig, err := bundle(gocontext.Background(), "../internal/data", &lazyTempDir{root: t.TempDir()}, mockObserver{}) + functions, schedules, functionsConfig, err := bundle(gocontext.Background(), testDir(t, "../internal/data"), newTestTempDir(t), mockObserver{}) assert.Nil(t, err) assert.Equal(t, 1, len(schedules)) @@ -1005,7 +1025,7 @@ func TestBundleWithManifestEventSubscriptions(t *testing.T) { assert.Nil(t, err) defer manifestFileHandle.Close() - _, _, functionsConfig, err := bundleFromManifest(gocontext.Background(), manifestFileHandle, &lazyTempDir{root: t.TempDir()}, mockObserver{}) + _, _, functionsConfig, err := bundleFromManifest(gocontext.Background(), testDir(t, basePath), manifestFileHandle, newTestTempDir(t), mockObserver{}) assert.Nil(t, err) helloJSConfig := functionsConfig["hello-js-function-test"] @@ -1013,7 +1033,7 @@ func TestBundleWithManifestEventSubscriptions(t *testing.T) { } func TestReadZipRuntime(t *testing.T) { - runtime, err := readZipRuntime("../internal/data/hello-rs-function-test.zip") + runtime, err := readZipRuntime(testDir(t, "../internal/data").root, "hello-rs-function-test.zip") assert.Nil(t, err) assert.Equal(t, "rs", runtime) diff --git a/go/porcelain/deploy_windows.go b/go/porcelain/deploy_windows.go index 02d280d3..6006b76b 100644 --- a/go/porcelain/deploy_windows.go +++ b/go/porcelain/deploy_windows.go @@ -2,12 +2,5 @@ package porcelain -import ( - "fmt" - "os" - "strings" -) - -func forceSlashSeparators(name string) string { - return strings.Replace(name, fmt.Sprintf("%c", os.PathSeparator), "/", -1) -} +// Windows has no O_NONBLOCK and no FIFOs to guard against. +const openNonblock = 0 diff --git a/go/porcelain/deploy_windows_test.go b/go/porcelain/deploy_windows_test.go deleted file mode 100644 index f10e75db..00000000 --- a/go/porcelain/deploy_windows_test.go +++ /dev/null @@ -1,10 +0,0 @@ -package porcelain - -import "testing" - -func TestForceSlashSeparators(t *testing.T) { - out := forceSlashSeparators("foo\\bar\\baz.js") - if out != "foo/bar/baz.js" { - t.Fatalf("expected `foo/bar/baz.js`, got `%s`", out) - } -} From 0c39701ab31b59239c0fcc6c58edde2c46face8c Mon Sep 17 00:00:00 2001 From: pieh Date: Mon, 31 Aug 2026 16:39:12 +0200 Subject: [PATCH 4/4] address coderabbit --- go/porcelain/deploy.go | 8 ++++---- go/porcelain/deploy_containment_test.go | 3 +-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/go/porcelain/deploy.go b/go/porcelain/deploy.go index f9fb07de..bf68d590 100644 --- a/go/porcelain/deploy.go +++ b/go/porcelain/deploy.go @@ -974,7 +974,7 @@ func (l *lazyTempDir) get() (string, *os.Root, error) { } handle, err := os.OpenRoot(path) if err != nil { - os.RemoveAll(path) + _ = os.RemoveAll(path) return "", nil, err } l.path, l.handle, l.created = path, handle, true @@ -985,7 +985,7 @@ func (l *lazyTempDir) get() (string, *os.Root, error) { func (l *lazyTempDir) remove() { if l.created { _ = l.handle.Close() - os.RemoveAll(l.path) + _ = os.RemoveAll(l.path) } } @@ -999,7 +999,7 @@ func bundle(ctx context.Context, functionsDir dirHandle, tmpDir *lazyTempDir, ob // If a `manifest.json` file is found, we extract the functions and their // metadata from it. if err == nil { - defer manifestFile.Close() + defer func() { _ = manifestFile.Close() }() return bundleFromManifest(ctx, functionsDir, manifestFile, tmpDir, observer) } @@ -1325,7 +1325,7 @@ func bundleEdgeFunctions(ctx context.Context, edgeFunctionsDir dirHandle, observ return nil, err } manifestBytes, err := io.ReadAll(manifestFile) - manifestFile.Close() + _ = manifestFile.Close() if err != nil { return nil, err } diff --git a/go/porcelain/deploy_containment_test.go b/go/porcelain/deploy_containment_test.go index 2f302a33..3f76896b 100644 --- a/go/porcelain/deploy_containment_test.go +++ b/go/porcelain/deploy_containment_test.go @@ -3,7 +3,6 @@ package porcelain import ( gocontext "context" "encoding/json" - "fmt" "os" "path/filepath" "testing" @@ -112,5 +111,5 @@ func contentOf(t *testing.T, dir, name string) string { t.Helper() b, err := os.ReadFile(filepath.Join(dir, filepath.FromSlash(name))) require.NoError(t, err) - return fmt.Sprintf("%s", b) + return string(b) }