Skip to content

Commit 3b91346

Browse files
sbaker617vitess-bot[bot]timvaillancourtmhamza15
committed
[release-22.0] Restore: make loading compressor commands from MANIFEST opt-in (vitessio#19460) (vitessio#19473)
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> Co-authored-by: Tim Vaillancourt <tim@timvaillancourt.com> Co-authored-by: Mohamed Hamza <mhamza@fastmail.com> Signed-off-by: Steve Baker <s.baker@slack-corp.com>
1 parent e5a1a93 commit 3b91346

11 files changed

Lines changed: 122 additions & 9 deletions

File tree

go/flags/endtoend/vtbackup.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@ Flags:
131131
--external-compressor string command with arguments to use when compressing a backup.
132132
--external-compressor-extension string extension to use when using an external compressor.
133133
--external-decompressor string command with arguments to use when decompressing a backup.
134+
--external-decompressor-use-manifest allows the decompressor command stored in the backup manifest to be used at restore time. Enabling this is a security risk: an attacker with write access to the backup storage could modify the manifest to execute arbitrary commands on the tablet as the Vitess user. NOT RECOMMENDED.
134135
--file_backup_storage_root string Root directory for the file backup storage.
135136
--gcs_backup_storage_bucket string Google Cloud Storage bucket to use for backups.
136137
--gcs_backup_storage_root string Root prefix for all backup-related object names.

go/flags/endtoend/vtcombo.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ Flags:
132132
--external-compressor string command with arguments to use when compressing a backup.
133133
--external-compressor-extension string extension to use when using an external compressor.
134134
--external-decompressor string command with arguments to use when decompressing a backup.
135+
--external-decompressor-use-manifest allows the decompressor command stored in the backup manifest to be used at restore time. Enabling this is a security risk: an attacker with write access to the backup storage could modify the manifest to execute arbitrary commands on the tablet as the Vitess user. NOT RECOMMENDED.
135136
--external_topo_server Should vtcombo use an external topology server instead of starting its own in-memory topology server. If true, vtcombo will use the flags defined in topo/server.go to open topo server
136137
--foreign_key_mode string This is to provide how to handle foreign key constraint in create/alter table. Valid values are: allow, disallow (default "allow")
137138
--gate_query_cache_memory int gate server query cache size in bytes, maximum amount of memory to be cached. vtgate analyzes every incoming query and generate a query plan, these plans are being cached in a lru cache. This config controls the capacity of the lru cache. (default 33554432)

go/flags/endtoend/vttablet.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,7 @@ Flags:
156156
--external-compressor string command with arguments to use when compressing a backup.
157157
--external-compressor-extension string extension to use when using an external compressor.
158158
--external-decompressor string command with arguments to use when decompressing a backup.
159+
--external-decompressor-use-manifest allows the decompressor command stored in the backup manifest to be used at restore time. Enabling this is a security risk: an attacker with write access to the backup storage could modify the manifest to execute arbitrary commands on the tablet as the Vitess user. NOT RECOMMENDED.
159160
--file_backup_storage_root string Root directory for the file backup storage.
160161
--filecustomrules string file based custom rule path
161162
--filecustomrules_watch set up a watch on the target file and reload query rules when it changes

go/flags/endtoend/vttestserver.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ Flags:
3838
--external-compressor string command with arguments to use when compressing a backup.
3939
--external-compressor-extension string extension to use when using an external compressor.
4040
--external-decompressor string command with arguments to use when decompressing a backup.
41+
--external-decompressor-use-manifest allows the decompressor command stored in the backup manifest to be used at restore time. Enabling this is a security risk: an attacker with write access to the backup storage could modify the manifest to execute arbitrary commands on the tablet as the Vitess user. NOT RECOMMENDED.
4142
--external_topo_global_root string the path of the global topology data in the global topology server for vtcombo process
4243
--external_topo_global_server_address string the address of the global topology server for vtcombo process
4344
--external_topo_implementation string the topology implementation to use for vtcombo process

go/test/endtoend/backup/vtctlbackup/backup_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ func TestBuiltinBackupWithExternalZstdCompressionAndManifestedDecompressor(t *te
6060
CompressorEngineName: "external",
6161
ExternalCompressorCmd: "zstd",
6262
ExternalCompressorExt: ".zst",
63+
ExternalDecompressorUseManifest: true,
6364
ManifestExternalDecompressorCmd: "zstd -d",
6465
}
6566

go/test/endtoend/backup/vtctlbackup/backup_utils.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ type CompressionDetails struct {
9292
ExternalCompressorCmd string
9393
ExternalCompressorExt string
9494
ExternalDecompressorCmd string
95+
ExternalDecompressorUseManifest bool
9596
ManifestExternalDecompressorCmd string
9697
}
9798

@@ -305,6 +306,9 @@ func getCompressorArgs(cDetails *CompressionDetails) []string {
305306
if cDetails.ExternalDecompressorCmd != "" {
306307
args = append(args, fmt.Sprintf("--external-decompressor=%s", cDetails.ExternalDecompressorCmd))
307308
}
309+
if cDetails.ExternalDecompressorUseManifest {
310+
args = append(args, "--external-decompressor-use-manifest")
311+
}
308312
if cDetails.ManifestExternalDecompressorCmd != "" {
309313
args = append(args, fmt.Sprintf("--manifest-external-decompressor=%s", cDetails.ManifestExternalDecompressorCmd))
310314
}

go/test/endtoend/backup/xtrabackup/xtrabackup_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ func TestXtrabackupWithExternalZstdCompressionAndManifestedDecompressor(t *testi
5959
CompressorEngineName: "external",
6060
ExternalCompressorCmd: "zstd",
6161
ExternalCompressorExt: ".zst",
62+
ExternalDecompressorUseManifest: true,
6263
ManifestExternalDecompressorCmd: "zstd -d",
6364
}
6465

go/vt/mysqlctl/builtinbackupengine.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1334,10 +1334,7 @@ func (be *BuiltinBackupEngine) restoreFile(ctx context.Context, params RestorePa
13341334
// for backward compatibility
13351335
deCompressionEngine = PgzipCompressor
13361336
}
1337-
externalDecompressorCmd := ExternalDecompressorCmd
1338-
if externalDecompressorCmd == "" && bm.ExternalDecompressor != "" {
1339-
externalDecompressorCmd = bm.ExternalDecompressor
1340-
}
1337+
externalDecompressorCmd := resolveExternalDecompressor(bm.ExternalDecompressor)
13411338
if externalDecompressorCmd != "" {
13421339
if deCompressionEngine == ExternalCompressor {
13431340
deCompressionEngine = externalDecompressorCmd

go/vt/mysqlctl/compression.go

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,9 +52,10 @@ var (
5252
ExternalCompressorCmd string
5353
ExternalCompressorExt string
5454
ExternalDecompressorCmd string
55+
ExternalDecompressorUseManifest bool
5556
ManifestExternalDecompressorCmd string
5657

57-
errUnsupportedDeCompressionEngine = errors.New("unsupported engine in MANIFEST. You need to provide --external-decompressor if using 'external' compression engine")
58+
errUnsupportedDeCompressionEngine = errors.New("unsupported engine in MANIFEST. You need to provide --external-decompressor if using 'external' compression engine. Alternatively, set --external-decompressor-use-manifest to use the decompressor command from the backup manifest, but this is NOT RECOMMENDED as it is a security risk")
5859
errUnsupportedCompressionEngine = errors.New("unsupported engine value for --compression-engine-name. supported values are 'external', 'pgzip', 'pargzip', 'zstd', 'lz4'")
5960

6061
// this is used by getEngineFromExtension() to figure out which engine to use in case the user didn't specify
@@ -77,6 +78,7 @@ func registerBackupCompressionFlags(fs *pflag.FlagSet) {
7778
fs.StringVar(&ExternalCompressorCmd, "external-compressor", ExternalCompressorCmd, "command with arguments to use when compressing a backup.")
7879
fs.StringVar(&ExternalCompressorExt, "external-compressor-extension", ExternalCompressorExt, "extension to use when using an external compressor.")
7980
fs.StringVar(&ExternalDecompressorCmd, "external-decompressor", ExternalDecompressorCmd, "command with arguments to use when decompressing a backup.")
81+
fs.BoolVar(&ExternalDecompressorUseManifest, "external-decompressor-use-manifest", ExternalDecompressorUseManifest, "allows the decompressor command stored in the backup manifest to be used at restore time. Enabling this is a security risk: an attacker with write access to the backup storage could modify the manifest to execute arbitrary commands on the tablet as the Vitess user. NOT RECOMMENDED.")
8082
fs.StringVar(&ManifestExternalDecompressorCmd, "manifest-external-decompressor", ManifestExternalDecompressorCmd, "command with arguments to store in the backup manifest when compressing a backup with an external compression engine.")
8183
}
8284

@@ -91,6 +93,20 @@ func getExtensionFromEngine(engine string) (string, error) {
9193
return "", fmt.Errorf("%w %q", errUnsupportedCompressionEngine, engine)
9294
}
9395

96+
// resolveExternalDecompressor returns the external decompressor command to use
97+
// at restore time. The CLI flag (--external-decompressor) takes precedence. The
98+
// backup manifest value is only used when --external-decompressor-use-manifest
99+
// is explicitly set to true.
100+
func resolveExternalDecompressor(manifestDecompressor string) string {
101+
if ExternalDecompressorCmd != "" {
102+
return ExternalDecompressorCmd
103+
}
104+
if ExternalDecompressorUseManifest && manifestDecompressor != "" {
105+
return manifestDecompressor
106+
}
107+
return ""
108+
}
109+
94110
// Validates if the external decompressor exists and return its path.
95111
func validateExternalCmd(cmd string) (string, error) {
96112
if cmd == "" {
Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
/*
2+
Copyright 2026 The Vitess Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package mysqlctl
18+
19+
import (
20+
"testing"
21+
22+
"github.com/stretchr/testify/assert"
23+
)
24+
25+
func TestResolveExternalDecompressor(t *testing.T) {
26+
tests := []struct {
27+
name string
28+
cliDecompressorCmd string
29+
useManifest bool
30+
manifestDecompressor string
31+
expected string
32+
}{
33+
{
34+
name: "CLI flag takes precedence over manifest",
35+
cliDecompressorCmd: "zstd -d",
36+
useManifest: true,
37+
manifestDecompressor: "gzip -d",
38+
expected: "zstd -d",
39+
},
40+
{
41+
name: "CLI flag takes precedence even when use-manifest is false",
42+
cliDecompressorCmd: "zstd -d",
43+
useManifest: false,
44+
manifestDecompressor: "gzip -d",
45+
expected: "zstd -d",
46+
},
47+
{
48+
name: "manifest used when use-manifest is true and no CLI flag",
49+
cliDecompressorCmd: "",
50+
useManifest: true,
51+
manifestDecompressor: "gzip -d",
52+
expected: "gzip -d",
53+
},
54+
{
55+
name: "manifest ignored when use-manifest is false",
56+
cliDecompressorCmd: "",
57+
useManifest: false,
58+
manifestDecompressor: "gzip -d",
59+
expected: "",
60+
},
61+
{
62+
name: "empty when nothing is set",
63+
cliDecompressorCmd: "",
64+
useManifest: false,
65+
manifestDecompressor: "",
66+
expected: "",
67+
},
68+
{
69+
name: "empty when use-manifest is true but manifest is empty",
70+
cliDecompressorCmd: "",
71+
useManifest: true,
72+
manifestDecompressor: "",
73+
expected: "",
74+
},
75+
}
76+
77+
for _, tt := range tests {
78+
t.Run(tt.name, func(t *testing.T) {
79+
origCmd := ExternalDecompressorCmd
80+
origAllow := ExternalDecompressorUseManifest
81+
t.Cleanup(func() {
82+
ExternalDecompressorCmd = origCmd
83+
ExternalDecompressorUseManifest = origAllow
84+
})
85+
86+
ExternalDecompressorCmd = tt.cliDecompressorCmd
87+
ExternalDecompressorUseManifest = tt.useManifest
88+
89+
result := resolveExternalDecompressor(tt.manifestDecompressor)
90+
assert.Equal(t, tt.expected, result)
91+
})
92+
}
93+
}

0 commit comments

Comments
 (0)