Skip to content

Sanitize credentials from snapshot repository URLs#1174

Open
arska wants to merge 1 commit intomasterfrom
fix/sanitize-snapshot-repo-url-1077
Open

Sanitize credentials from snapshot repository URLs#1174
arska wants to merge 1 commit intomasterfrom
fix/sanitize-snapshot-repo-url-1077

Conversation

@arska
Copy link
Copy Markdown
Member

@arska arska commented Mar 22, 2026

Summary

REST backend repository URLs can contain credentials in the userinfo component (e.g. rest:https://user:pass@host/path). These were stored verbatim in Snapshot CRs and visible via kubectl get snapshot, exposing credentials to anyone with read access to the namespace.

Before

$ kubectl get snapshot
NAME       DATE TAKEN             PATHS   REPOSITORY
ed134283   2025-07-08T01:00:05Z   /data   rest:https://backup:secret@backup.example.com/repo

After

$ kubectl get snapshot
NAME       DATE TAKEN             PATHS   REPOSITORY
ed134283   2025-07-08T01:00:05Z   /data   rest:https://redacted:redacted@backup.example.com/repo

Only REST backends are affected — S3/Azure/Swift/B2/GCS backends don't include credentials in the repository URL.

Fixes #1077

Checklist

  • Commits are signed off
  • PR contains the label area:operator
  • I have not made any changes in the charts/ directory

Test plan

  • Added unit tests for URL sanitization (all backend types)
  • All existing snapshot tests pass

🤖 Generated with Claude Code

@arska arska requested a review from a team as a code owner March 22, 2026 22:12
@arska arska added bug Something isn't working area:operator labels Mar 22, 2026
@arska arska requested review from Kidswiss and lieneluksika and removed request for a team March 22, 2026 22:12
@arska arska self-assigned this Mar 22, 2026
@arska arska added bug Something isn't working area:operator labels Mar 22, 2026
Copy link
Copy Markdown
Contributor

@tobru tobru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Kidswiss Mind taking another look?

@arska arska force-pushed the fix/sanitize-snapshot-repo-url-1077 branch from f5f0306 to 8d5cbdb Compare March 23, 2026 11:58
@arska arska removed the request for review from lieneluksika March 23, 2026 12:41
@arska arska force-pushed the fix/sanitize-snapshot-repo-url-1077 branch from 8d5cbdb to aff1e1d Compare March 23, 2026 13:41
REST backend repository URLs can contain credentials in the
userinfo component (e.g. rest:https://user:pass@host/path).
These were stored verbatim in Snapshot CRs and visible via
kubectl get snapshot, exposing credentials to anyone with
read access to the namespace.

Add sanitizeRepositoryURL() that replaces userinfo with
"redacted" before storing in the Snapshot spec. This only
affects REST backends — S3/Azure/Swift/B2/GCS backends don't
include credentials in the repository URL.

Fixes #1077

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Aarno Aukia <aarno.aukia@vshn.ch>
@arska arska force-pushed the fix/sanitize-snapshot-repo-url-1077 branch from aff1e1d to 498d8c5 Compare March 23, 2026 13:57
Comment on lines +26 to +33
if idx := strings.Index(repo, ":"); idx > 0 {
candidate := repo[:idx]
// Check if it looks like a restic scheme (not a Windows drive letter or URL scheme)
if candidate == "rest" || candidate == "s3" || candidate == "azure" || candidate == "swift" || candidate == "b2" || candidate == "gs" {
prefix = repo[:idx+1]
rest = repo[idx+1:]
}
}
Copy link
Copy Markdown
Member

@bastjan bastjan Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if idx := strings.Index(repo, ":"); idx > 0 {
candidate := repo[:idx]
// Check if it looks like a restic scheme (not a Windows drive letter or URL scheme)
if candidate == "rest" || candidate == "s3" || candidate == "azure" || candidate == "swift" || candidate == "b2" || candidate == "gs" {
prefix = repo[:idx+1]
rest = repo[idx+1:]
}
}
if candidate, after, found := strings.Cut(repo, ":"); found {
// Check if it looks like a restic scheme (not a Windows drive letter or URL scheme)
if candidate == "rest" || candidate == "s3" || candidate == "azure" || candidate == "swift" || candidate == "b2" || candidate == "gs" {
prefix = candidate + ":"
rest = after
}
}

Much easier to understand IMO.

"local path": {
input: "/tmp/restic-repo",
expected: "/tmp/restic-repo",
},
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
},
},
"scheme only": {
input: "s3:",
expected: "s3:",
},
"windows drive letter": {
input: "C:\\path\\to\\repo",
expected: "C:\\path\\to\\repo",
},
"url scheme": {
input: "http://example.com/repo",
expected: "http://example.com/repo",
},

We probably should test what was written in the comment.

Copy link
Copy Markdown
Contributor

@Kidswiss Kidswiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please implement bastjan's suggestions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:operator bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sensitive credentials exposed in plain text in snapshot resource output

4 participants