Skip to content

Commit 17ae5ca

Browse files
GMHDBJDGMHDBJD
authored
importsdk: redact sensitive source params in outward-facing errors (pingcap#67719)
close pingcap#67718
1 parent a9774df commit 17ae5ca

3 files changed

Lines changed: 95 additions & 19 deletions

File tree

‎pkg/importsdk/BUILD.bazel‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ go_test(
5858
"//pkg/util/table-filter",
5959
"@com_github_data_dog_go_sqlmock//:go-sqlmock",
6060
"@com_github_fsouza_fake_gcs_server//fakestorage",
61+
"@com_github_go_sql_driver_mysql//:mysql",
6162
"@com_github_ngaut_pools//:pools",
6263
"@com_github_stretchr_testify//require",
6364
"@com_github_stretchr_testify//suite",

‎pkg/importsdk/file_scanner.go‎

Lines changed: 28 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -48,23 +48,32 @@ type FileScanner interface {
4848
}
4949

5050
type fileScanner struct {
51-
sourcePath string
52-
db *sql.DB
53-
store storeapi.Storage
54-
loader *mydump.MDLoader
55-
logger log.Logger
56-
config *SDKConfig
51+
redactedSourcePath string
52+
db *sql.DB
53+
store storeapi.Storage
54+
loader *mydump.MDLoader
55+
logger log.Logger
56+
config *SDKConfig
5757
}
5858

59+
const redactedInvalidSourcePath = "<redacted-invalid-source>"
60+
5961
// NewFileScanner creates a new FileScanner
6062
func NewFileScanner(ctx context.Context, sourcePath string, db *sql.DB, cfg *SDKConfig) (FileScanner, error) {
63+
redactedSourcePath := ast.RedactURL(sourcePath)
64+
parseErrorSourcePath := redactedSourcePath
65+
if parseErrorSourcePath == sourcePath {
66+
// ast.RedactURL leaves malformed or unsupported URLs unchanged.
67+
// Avoid exposing the original source in outward-facing parse errors.
68+
parseErrorSourcePath = redactedInvalidSourcePath
69+
}
6170
u, err := objstore.ParseBackend(sourcePath, nil)
6271
if err != nil {
63-
return nil, errors.Annotatef(ErrParseStorageURL, "source=%s, err=%v", sourcePath, err)
72+
return nil, errors.Annotatef(ErrParseStorageURL, "source=%s", parseErrorSourcePath)
6473
}
6574
store, err := objstore.New(ctx, u, &storeapi.Options{})
6675
if err != nil {
67-
return nil, errors.Annotatef(ErrCreateExternalStorage, "source=%s, err=%v", sourcePath, err)
76+
return nil, errors.Annotatef(ErrCreateExternalStorage, "source=%s, err=%v", redactedSourcePath, err)
6877
}
6978

7079
ldrCfg := mydump.LoaderConfig{
@@ -90,24 +99,24 @@ func NewFileScanner(ctx context.Context, sourcePath string, db *sql.DB, cfg *SDK
9099
loader, err := mydump.NewLoaderWithStore(ctx, ldrCfg, store, loaderOptions...)
91100
if err != nil {
92101
if loader == nil || !errors.ErrorEqual(err, common.ErrTooManySourceFiles) {
93-
return nil, errors.Annotatef(ErrCreateLoader, "source=%s, charset=%s, err=%v", sourcePath, cfg.charset, err)
102+
return nil, errors.Annotatef(ErrCreateLoader, "source=%s, charset=%s, err=%v", redactedSourcePath, cfg.charset, err)
94103
}
95104
}
96105

97106
return &fileScanner{
98-
sourcePath: sourcePath,
99-
db: db,
100-
store: store,
101-
loader: loader,
102-
logger: cfg.logger,
103-
config: cfg,
107+
redactedSourcePath: redactedSourcePath,
108+
db: db,
109+
store: store,
110+
loader: loader,
111+
logger: cfg.logger,
112+
config: cfg,
104113
}, nil
105114
}
106115

107116
func (s *fileScanner) CreateSchemasAndTables(ctx context.Context) error {
108117
dbMetas := s.loader.GetDatabases()
109118
if len(dbMetas) == 0 {
110-
return errors.Annotatef(ErrNoDatabasesFound, "source=%s", s.sourcePath)
119+
return errors.Annotatef(ErrNoDatabasesFound, "source=%s", s.redactedSourcePath)
111120
}
112121

113122
// Create all schemas and tables
@@ -121,7 +130,7 @@ func (s *fileScanner) CreateSchemasAndTables(ctx context.Context) error {
121130

122131
err := importer.Run(ctx, dbMetas)
123132
if err != nil {
124-
return errors.Annotatef(ErrCreateSchema, "source=%s, db_count=%d, err=%v", s.sourcePath, len(dbMetas), err)
133+
return errors.Annotatef(ErrCreateSchema, "source=%s, db_count=%d, err=%v", s.redactedSourcePath, len(dbMetas), err)
125134
}
126135

127136
return nil
@@ -155,7 +164,7 @@ func (s *fileScanner) CreateSchemaAndTableByName(ctx context.Context, schema, ta
155164
Tables: []*mydump.MDTableMeta{tblMeta},
156165
}})
157166
if err != nil {
158-
return errors.Annotatef(ErrCreateSchema, "source=%s, schema=%s, table=%s, err=%v", s.sourcePath, schema, table, err)
167+
return errors.Annotatef(ErrCreateSchema, "source=%s, schema=%s, table=%s, err=%v", s.redactedSourcePath, schema, table, err)
159168
}
160169

161170
return nil
@@ -202,7 +211,7 @@ func (s *fileScanner) GetTotalSize(ctx context.Context) int64 {
202211
func (s *fileScanner) EstimateImportDataSize(ctx context.Context) (*ImportDataSizeEstimate, error) {
203212
dbMetas := s.loader.GetDatabases()
204213
if len(dbMetas) == 0 {
205-
return nil, errors.Annotatef(ErrNoDatabasesFound, "source=%s", s.sourcePath)
214+
return nil, errors.Annotatef(ErrNoDatabasesFound, "source=%s", s.redactedSourcePath)
206215
}
207216

208217
result := &ImportDataSizeEstimate{

‎pkg/importsdk/file_scanner_test.go‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,11 @@ import (
2626
"testing"
2727

2828
"github.com/DATA-DOG/go-sqlmock"
29+
dmysql "github.com/go-sql-driver/mysql"
2930
"github.com/pingcap/tidb/pkg/lightning/config"
3031
"github.com/pingcap/tidb/pkg/lightning/mydump"
32+
"github.com/pingcap/tidb/pkg/parser/ast"
33+
tmysql "github.com/pingcap/tidb/pkg/parser/mysql"
3134
filter "github.com/pingcap/tidb/pkg/util/table-filter"
3235
"github.com/stretchr/testify/require"
3336
)
@@ -68,6 +71,16 @@ func TestProcessDataFiles(t *testing.T) {
6871
func TestFileScanner(t *testing.T) {
6972
tmpDir := t.TempDir()
7073
ctx := context.Background()
74+
assertSecretsRedacted := func(t *testing.T, err error) {
75+
t.Helper()
76+
require.Error(t, err)
77+
require.ErrorContains(t, err, "access-key=xxxxxx")
78+
require.ErrorContains(t, err, "secret-access-key=xxxxxx")
79+
require.ErrorContains(t, err, "session-token=xxxxxx")
80+
require.NotContains(t, err.Error(), "access-key=ak")
81+
require.NotContains(t, err.Error(), "secret-access-key=sk")
82+
require.NotContains(t, err.Error(), "session-token=token")
83+
}
7184

7285
require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "db1-schema-create.sql"), []byte("CREATE DATABASE db1;"), 0644))
7386
require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "db1.t1-schema.sql"), []byte("CREATE TABLE t1 (id INT);"), 0644))
@@ -117,6 +130,59 @@ func TestFileScanner(t *testing.T) {
117130
require.NoError(t, mock.ExpectationsWereMet())
118131
})
119132

133+
t.Run("NewFileScannerRedactsSensitiveSourcePathInParseErrors", func(t *testing.T) {
134+
_, err := NewFileScanner(
135+
ctx,
136+
"s3://?access-key=ak&secret-access-key=sk&session-token=token",
137+
db,
138+
cfg,
139+
)
140+
assertSecretsRedacted(t, err)
141+
})
142+
143+
t.Run("NewFileScannerHidesMalformedSensitiveSourcePathInParseErrors", func(t *testing.T) {
144+
_, err := NewFileScanner(
145+
ctx,
146+
"1invalid:?secret-access-key=sk",
147+
db,
148+
cfg,
149+
)
150+
require.Error(t, err)
151+
require.ErrorContains(t, err, "source="+redactedInvalidSourcePath)
152+
require.NotContains(t, err.Error(), "secret-access-key=sk")
153+
})
154+
155+
t.Run("CreateSchemasAndTablesRedactsSensitiveSourcePathOnError", func(t *testing.T) {
156+
invalidDir := t.TempDir()
157+
require.NoError(t, os.WriteFile(filepath.Join(invalidDir, "db1-schema-create.sql"), []byte("CREATE DATABASE db1;"), 0o644))
158+
require.NoError(t, os.WriteFile(
159+
filepath.Join(invalidDir, "db1.t1-schema.sql"),
160+
[]byte("CREATE TABLE t1 (id INT,);"),
161+
0o644,
162+
))
163+
164+
invalidDB, invalidMock, err := sqlmock.New()
165+
require.NoError(t, err)
166+
defer invalidDB.Close()
167+
168+
invalidScanner, err := NewFileScanner(ctx, "file://"+invalidDir, invalidDB, defaultSDKConfig())
169+
require.NoError(t, err)
170+
defer invalidScanner.Close()
171+
172+
fs := invalidScanner.(*fileScanner)
173+
sourcePath := "s3://bucket/path?access-key=ak&secret-access-key=sk&session-token=token"
174+
fs.redactedSourcePath = ast.RedactURL(sourcePath)
175+
176+
invalidMock.ExpectQuery("SELECT SCHEMA_NAME FROM information_schema.SCHEMATA.*").WillReturnRows(sqlmock.NewRows([]string{"SCHEMA_NAME"}))
177+
invalidMock.ExpectExec(regexp.QuoteMeta("CREATE DATABASE IF NOT EXISTS `db1`")).WillReturnResult(sqlmock.NewResult(0, 0))
178+
invalidMock.ExpectQuery("SHOW CREATE TABLE `db1`.`t1`").WillReturnError(&dmysql.MySQLError{Number: tmysql.ErrNoSuchTable})
179+
180+
err = invalidScanner.CreateSchemasAndTables(ctx)
181+
assertSecretsRedacted(t, err)
182+
require.ErrorContains(t, err, "invalid schema statement")
183+
require.NoError(t, invalidMock.ExpectationsWereMet())
184+
})
185+
120186
t.Run("CreateSchemaAndTableByName", func(t *testing.T) {
121187
mock.ExpectQuery("SELECT SCHEMA_NAME FROM information_schema.SCHEMATA.*").WillReturnRows(sqlmock.NewRows([]string{"SCHEMA_NAME"}))
122188
mock.ExpectExec(regexp.QuoteMeta("CREATE DATABASE IF NOT EXISTS `db1`")).WillReturnResult(sqlmock.NewResult(0, 0))

0 commit comments

Comments
 (0)