Better handling of clues, error, and log data if it's PII (#3935)

Add better handling for
* hiding possibly sensitive data when logging
* adding possibly sensitive data to context clues
* adding context clues to returned errors

---

#### Does this PR need a docs update or release note?

- [ ]  Yes, it's included
- [ ] 🕐 Yes, but in a later PR
- [x]  No

#### Type of change

- [ ] 🌻 Feature
- [ ] 🐛 Bugfix
- [ ] 🗺️ Documentation
- [ ] 🤖 Supportability/Tests
- [ ] 💻 CI/Deployment
- [x] 🧹 Tech Debt/Cleanup

#### Issue(s)

* #3895

#### Test Plan

- [x] 💪 Manual
- [ ]  Unit test
- [ ] 💚 E2E
This commit is contained in:
ashmrtn 2023-07-31 15:00:20 -07:00 committed by GitHub
parent dfbca1540d
commit 5b455bfc4a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 35 additions and 25 deletions

View File

@ -252,7 +252,9 @@ func (cp *corsoProgress) FinishedHashingFile(fname string, bs int64) {
sl[i] = string(rdt) sl[i] = string(rdt)
} }
logger.Ctx(context.Background()).Debugw("finished hashing file", "path", sl[2:]) logger.Ctx(cp.ctx).Debugw(
"finished hashing file",
"path", clues.Hide(path.Elements(sl[2:])))
atomic.AddInt64(&cp.totalBytes, bs) atomic.AddInt64(&cp.totalBytes, bs)
} }
@ -442,12 +444,12 @@ func streamBaseEntries(
ctx = clues.Add( ctx = clues.Add(
ctx, ctx,
"current_item_path", curPath, "current_directory_path", curPath,
"longest_prefix", longest) "longest_prefix", path.LoggableDir(longest))
err := dir.IterateEntries(ctx, func(innerCtx context.Context, entry fs.Entry) error { err := dir.IterateEntries(ctx, func(innerCtx context.Context, entry fs.Entry) error {
if err := innerCtx.Err(); err != nil { if err := innerCtx.Err(); err != nil {
return err return clues.Stack(err).WithClues(ctx)
} }
// Don't walk subdirectories in this function. // Don't walk subdirectories in this function.
@ -464,7 +466,9 @@ func streamBaseEntries(
entName, err := decodeElement(entry.Name()) entName, err := decodeElement(entry.Name())
if err != nil { if err != nil {
return clues.Wrap(err, "decoding entry name: "+entry.Name()) return clues.Wrap(err, "decoding entry name").
WithClues(ctx).
With("entry_name", entry.Name())
} }
// This entry was marked as deleted by a service that can't tell us the // This entry was marked as deleted by a service that can't tell us the
@ -476,7 +480,7 @@ func streamBaseEntries(
// For now assuming that item IDs don't need escaping. // For now assuming that item IDs don't need escaping.
itemPath, err := curPath.AppendItem(entName) itemPath, err := curPath.AppendItem(entName)
if err != nil { if err != nil {
return clues.Wrap(err, "getting full item path for base entry") return clues.Wrap(err, "getting full item path for base entry").WithClues(ctx)
} }
// We need the previous path so we can find this item in the base snapshot's // We need the previous path so we can find this item in the base snapshot's
@ -485,7 +489,7 @@ func streamBaseEntries(
// to look for. // to look for.
prevItemPath, err := prevPath.AppendItem(entName) prevItemPath, err := prevPath.AppendItem(entName)
if err != nil { if err != nil {
return clues.Wrap(err, "getting previous full item path for base entry") return clues.Wrap(err, "getting previous full item path for base entry").WithClues(ctx)
} }
// Meta files aren't in backup details since it's the set of items the user // Meta files aren't in backup details since it's the set of items the user
@ -509,13 +513,15 @@ func streamBaseEntries(
} }
if err := ctr(ctx, entry); err != nil { if err := ctr(ctx, entry); err != nil {
return clues.Wrap(err, "executing callback on item").With("item_path", itemPath) return clues.Wrap(err, "executing callback on item").
WithClues(ctx).
With("item_path", itemPath)
} }
return nil return nil
}) })
if err != nil { if err != nil {
return clues.Wrap(err, "traversing items in base snapshot directory") return clues.Wrap(err, "traversing items in base snapshot directory").WithClues(ctx)
} }
return nil return nil
@ -826,7 +832,9 @@ func inflateCollectionTree(
} }
if node.collection != nil && node.collection.State() == data.NotMovedState { if node.collection != nil && node.collection.State() == data.NotMovedState {
return nil, nil, clues.New("conflicting states for collection").With("changed_path", p) return nil, nil, clues.New("conflicting states for collection").
WithClues(ctx).
With("changed_path", p)
} }
} }
@ -860,7 +868,7 @@ func traverseBaseDir(
"expected_dir_path", expectedDirPath) "expected_dir_path", expectedDirPath)
if depth >= maxInflateTraversalDepth { if depth >= maxInflateTraversalDepth {
return clues.New("base snapshot tree too tall") return clues.New("base snapshot tree too tall").WithClues(ctx)
} }
// Wrapper base64 encodes all file and folder names to avoid issues with // Wrapper base64 encodes all file and folder names to avoid issues with
@ -868,7 +876,9 @@ func traverseBaseDir(
// from kopia we need to do the decoding here. // from kopia we need to do the decoding here.
dirName, err := decodeElement(dir.Name()) dirName, err := decodeElement(dir.Name())
if err != nil { if err != nil {
return clues.Wrap(err, "decoding base directory name").With("dir_name", dir.Name()) return clues.Wrap(err, "decoding base directory name").
WithClues(ctx).
With("dir_name", clues.Hide(dir.Name()))
} }
// Form the path this directory would be at if the hierarchy remained the same // Form the path this directory would be at if the hierarchy remained the same
@ -944,7 +954,7 @@ func traverseBaseDir(
stats) stats)
}) })
if err != nil { if err != nil {
return clues.Wrap(err, "traversing base directory") return clues.Wrap(err, "traversing base directory").WithClues(ctx)
} }
// We only need to add this base directory to the tree we're building if it // We only need to add this base directory to the tree we're building if it
@ -961,7 +971,7 @@ func traverseBaseDir(
// in the if-block though as that is an optimization. // in the if-block though as that is an optimization.
node := getTreeNode(roots, currentPath.Elements()) node := getTreeNode(roots, currentPath.Elements())
if node == nil { if node == nil {
return clues.New("getting tree node") return clues.New("getting tree node").WithClues(ctx)
} }
// Now that we have the node we need to check if there is a collection // Now that we have the node we need to check if there is a collection
@ -976,12 +986,12 @@ func traverseBaseDir(
curP, err := path.FromDataLayerPath(currentPath.String(), false) curP, err := path.FromDataLayerPath(currentPath.String(), false)
if err != nil { if err != nil {
return clues.New("converting current path to path.Path") return clues.New("converting current path to path.Path").WithClues(ctx)
} }
oldP, err := path.FromDataLayerPath(oldDirPath.String(), false) oldP, err := path.FromDataLayerPath(oldDirPath.String(), false)
if err != nil { if err != nil {
return clues.New("converting old path to path.Path") return clues.New("converting old path to path.Path").WithClues(ctx)
} }
node.baseDir = dir node.baseDir = dir
@ -1172,7 +1182,7 @@ func inflateDirTree(
} }
if len(roots) > 1 { if len(roots) > 1 {
return nil, clues.New("multiple root directories") return nil, clues.New("multiple root directories").WithClues(ctx)
} }
var res fs.Directory var res fs.Directory

View File

@ -324,7 +324,7 @@ func (w Wrapper) makeSnapshotWithRoot(
// Telling kopia to always flush may hide other errors if it fails while // Telling kopia to always flush may hide other errors if it fails while
// flushing the write session (hence logging above). // flushing the write session (hence logging above).
if err != nil { if err != nil {
return nil, clues.Wrap(err, "kopia backup") return nil, clues.Wrap(err, "kopia backup").WithClues(ctx)
} }
res := manifestToStats(man, progress, bc) res := manifestToStats(man, progress, bc)
@ -369,7 +369,7 @@ func getDir(
encodeElements(dirPath.PopFront().Elements()...)) encodeElements(dirPath.PopFront().Elements()...))
if err != nil { if err != nil {
if isErrEntryNotFound(err) { if isErrEntryNotFound(err) {
err = clues.Stack(data.ErrNotFound, err) err = clues.Stack(data.ErrNotFound, err).WithClues(ctx)
} }
return nil, clues.Wrap(err, "getting nested object handle").WithClues(ctx) return nil, clues.Wrap(err, "getting nested object handle").WithClues(ctx)
@ -487,7 +487,7 @@ func (w Wrapper) ProduceRestoreCollections(
// load it here. // load it here.
snapshotRoot, err := w.getSnapshotRoot(ctx, snapshotID) snapshotRoot, err := w.getSnapshotRoot(ctx, snapshotID)
if err != nil { if err != nil {
return nil, clues.Wrap(err, "loading snapshot root") return nil, clues.Wrap(err, "loading snapshot root").WithClues(ctx)
} }
var ( var (
@ -507,8 +507,8 @@ func (w Wrapper) ProduceRestoreCollections(
// items from a single directory instance lower down. // items from a single directory instance lower down.
ictx := clues.Add( ictx := clues.Add(
ctx, ctx,
"item_path", itemPaths.StoragePath.String(), "item_path", itemPaths.StoragePath,
"restore_path", itemPaths.RestorePath.String()) "restore_path", itemPaths.RestorePath)
parentStoragePath, err := itemPaths.StoragePath.Dir() parentStoragePath, err := itemPaths.StoragePath.Dir()
if err != nil { if err != nil {
@ -552,7 +552,7 @@ func (w Wrapper) ProduceRestoreCollections(
// then load the items from the directory. // then load the items from the directory.
res, err := loadDirsAndItems(ctx, snapshotRoot, bcounter, dirsToItems, errs) res, err := loadDirsAndItems(ctx, snapshotRoot, bcounter, dirsToItems, errs)
if err != nil { if err != nil {
return nil, clues.Wrap(err, "loading items") return nil, clues.Wrap(err, "loading items").WithClues(ctx)
} }
return res, el.Failure() return res, el.Failure()
@ -610,12 +610,12 @@ func (w Wrapper) RepoMaintenance(
) error { ) error {
kopiaSafety, err := translateSafety(opts.Safety) kopiaSafety, err := translateSafety(opts.Safety)
if err != nil { if err != nil {
return clues.Wrap(err, "identifying safety level") return clues.Wrap(err, "identifying safety level").WithClues(ctx)
} }
mode, err := translateMode(opts.Type) mode, err := translateMode(opts.Type)
if err != nil { if err != nil {
return clues.Wrap(err, "identifying maintenance mode") return clues.Wrap(err, "identifying maintenance mode").WithClues(ctx)
} }
currentOwner := w.c.ClientOptions().UsernameAtHost() currentOwner := w.c.ClientOptions().UsernameAtHost()