mirrors/velero
1
0
Fork 0
mirror of https://github.com/vmware-tanzu/velero.git synced 2025-12-23 06:15:21 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Address PR review comments: sanitize errors and add SAS token scrubbing

Browse Source 
This commit addresses three review comments on PR #9321:

1. Keep sanitization in controller (response to @ywk253100)
   - Maintaining centralized error handling for easier extension
   - Azure-specific patterns detected and others passed through unchanged

2. Sanitize unavailableErrors array (@priyansh17)
   - Now using sanitizeStorageError() for both unavailableErrors array
     and location.Status.Message for consistency

3. Add SAS token scrubbing (@anshulahuja98)
   - Scrubs Azure SAS token parameters to prevent credential leakage
   - Redacts: sig, se, st, sp, spr, sv, sr, sip, srt, ss
   - Example: ?sig=secret becomes ?sig=***REDACTED***

Added comprehensive test coverage for SAS token scrubbing with 4 new
test cases covering various scenarios.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Shubham Pampattiwar <spampatt@redhat.com>
This commit is contained in:
Shubham Pampattiwar
2025-11-12 15:39:38 -08:00
parent 60dd3dc832
commit 20af2c20c5
2 changed files with 42 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
pkg/controller/backup_storage_location_controller.go
View File

@@ -50,6 +50,7 @@ const (
// sanitizeStorageError cleans up verbose HTTP responses from cloud provider errors,
// particularly Azure which includes full HTTP response details and XML in error messages.
// It extracts the error code and message while removing HTTP headers and response bodies.
// It also scrubs sensitive information like SAS tokens from URLs.
func sanitizeStorageError(err error) string {
if err == nil {
return ""
@@ -57,6 +58,12 @@ func sanitizeStorageError(err error) string {
errMsg := err.Error()
// Scrub sensitive information from URLs (SAS tokens, credentials, etc.)
// Azure SAS token parameters: sig, se, st, sp, spr, sv, sr, sip, srt, ss
// These appear as query parameters in URLs like: ?sig=value&se=value
sasParamsRegex := regexp.MustCompile(`([?&])(sig|se|st|sp|spr|sv|sr|sip|srt|ss)=([^&\s<>\n]+)`)
errMsg = sasParamsRegex.ReplaceAllString(errMsg, `${1}${2}=***REDACTED***`)
// Check if this looks like an Azure HTTP response error
// Azure errors contain patterns like "RESPONSE 404:" and "ERROR CODE:"
if !strings.Contains(errMsg, "RESPONSE") || !strings.Contains(errMsg, "ERROR CODE:") {
@@ -217,7 +224,7 @@ func (r *backupStorageLocationReconciler) Reconcile(ctx context.Context, req ctr
if err != nil {
log.Info("BackupStorageLocation is invalid, marking as unavailable")
err = errors.Wrapf(err, "BackupStorageLocation %q is unavailable", location.Name)
unavailableErrors = append(unavailableErrors, err.Error())
unavailableErrors = append(unavailableErrors, sanitizeStorageError(err))
location.Status.Phase = velerov1api.BackupStorageLocationPhaseUnavailable
location.Status.Message = sanitizeStorageError(err)
} else {

34
pkg/controller/backup_storage_location_controller_test.go
View File

@@ -372,6 +372,40 @@ ERROR CODE: AuthorizationFailure
`),
expected: "rpc error: code = Unknown desc = AuthorizationFailure: Forbidden",
},
{
name: "Error with Azure SAS token in URL",
input: errors.New(`rpc error: code = Unknown desc = GET https://storage.blob.core.windows.net/backup?sv=2020-08-04&sig=abc123secrettoken&se=2024-12-31T23:59:59Z&sp=rwdl
--------------------------------------------------------------------------------
RESPONSE 404: 404 The specified container does not exist.
ERROR CODE: ContainerNotFound
--------------------------------------------------------------------------------
`),
expected: "rpc error: code = Unknown desc = ContainerNotFound: The specified container does not exist.",
},
{
name: "Error with multiple SAS parameters",
input: errors.New(`GET https://mystorageaccount.blob.core.windows.net/container?sv=2020-08-04&ss=b&srt=sco&sp=rwdlac&se=2024-12-31&st=2024-01-01&sip=168.1.5.60&spr=https&sig=SIGNATURE_HASH`),
expected: "GET https://mystorageaccount.blob.core.windows.net/container?sv=***REDACTED***&ss=***REDACTED***&srt=***REDACTED***&sp=***REDACTED***&se=***REDACTED***&st=***REDACTED***&sip=***REDACTED***&spr=***REDACTED***&sig=***REDACTED***",
},
{
name: "Simple URL without SAS tokens unchanged",
input: errors.New("GET https://storage.blob.core.windows.net/container/blob"),
expected: "GET https://storage.blob.core.windows.net/container/blob",
},
{
name: "Azure error with SAS token in full HTTP response",
input: errors.New(`rpc error: code = Unknown desc = GET https://oadp100711zl59k.blob.core.windows.net/backup?sig=secretsignature123&se=2024-12-31
--------------------------------------------------------------------------------
RESPONSE 404: 404 The specified container does not exist.
ERROR CODE: ContainerNotFound
--------------------------------------------------------------------------------
<?xml version="1.0" encoding="utf-8"?><Error><Code>ContainerNotFound</Code><Message>The specified container does not exist.
RequestId:63cf34d8-801e-0078-09b4-2e4682000000
Time:2024-11-04T12:23:04.5623627Z</Message></Error>
--------------------------------------------------------------------------------
`),
expected: "rpc error: code = Unknown desc = ContainerNotFound: The specified container does not exist.",
},
}
for _, test := range tests {