diff --git a/src/internal/connector/exchange/attachment.go b/src/internal/connector/exchange/attachment.go index 019699e77..f42f3348d 100644 --- a/src/internal/connector/exchange/attachment.go +++ b/src/internal/connector/exchange/attachment.go @@ -52,7 +52,7 @@ func uploadAttachment( ctx, "attachment_size", ptr.Val(attachment.GetSize()), "attachment_id", ptr.Val(attachment.GetId()), - "attachment_name", ptr.Val(attachment.GetName()), // TODO: pii + "attachment_name", clues.Hide(ptr.Val(attachment.GetName())), "attachment_type", attachmentType, "internal_item_type", getItemAttachmentItemType(attachment), "uploader_item_id", uploader.getItemID()) @@ -104,7 +104,8 @@ func uploadLargeAttachment( url := ptr.Val(session.GetUploadUrl()) aw := uploadsession.NewWriter(uploader.getItemID(), url, size) - logger.Ctx(ctx).Debugw("uploading large attachment", "attachment_url", url) // TODO: url pii + // TODO: url pii refinementt + logger.Ctx(ctx).Debugw("uploading large attachment", "attachment_url", clues.Hide(url)) // Upload the stream data copyBuffer := make([]byte, attachmentChunkSize) diff --git a/src/internal/connector/exchange/service_restore.go b/src/internal/connector/exchange/service_restore.go index c72b1cbf0..10808e7ed 100644 --- a/src/internal/connector/exchange/service_restore.go +++ b/src/internal/connector/exchange/service_restore.go @@ -314,7 +314,7 @@ func RestoreExchangeDataCollections( if len(dcs) > 0 { userID = dcs[0].FullPath().ResourceOwner() - ctx = clues.Add(ctx, "resource_owner", userID) // TODO: pii + ctx = clues.Add(ctx, "resource_owner", clues.Hide(userID)) } for _, dc := range dcs { diff --git a/src/internal/connector/graph/service.go b/src/internal/connector/graph/service.go index 93251b730..bbe6de0aa 100644 --- a/src/internal/connector/graph/service.go +++ b/src/internal/connector/graph/service.go @@ -280,7 +280,7 @@ func (handler *LoggingMiddleware) Intercept( ctx = clues.Add( req.Context(), "method", req.Method, - "url", req.URL, // TODO: pii + "url", req.URL, // TODO: pii, not hasing yet because we want debuggable urls "request_len", req.ContentLength, ) resp, err = pipeline.Next(req, middlewareIndex) diff --git a/src/internal/connector/graph_connector.go b/src/internal/connector/graph_connector.go index 692a9b83d..f3237ffec 100644 --- a/src/internal/connector/graph_connector.go +++ b/src/internal/connector/graph_connector.go @@ -165,16 +165,15 @@ func identifySite(item any) (string, string, error) { if m.GetName() == nil { // the built-in site at "https://{tenant-domain}/search" never has a name. if ok && strings.HasSuffix(url, "/search") { - // TODO: pii siteID, on this and all following cases - return "", "", clues.Stack(errKnownSkippableCase).With("site_id", id) + return "", "", clues.Stack(errKnownSkippableCase).With("site_id", clues.Hide(id)) } - return "", "", clues.New("site has no name").With("site_id", id) + return "", "", clues.New("site has no name").With("site_id", clues.Hide(id)) } // personal (ie: oneDrive) sites have to be filtered out server-side. if ok && strings.Contains(url, personalSitePath) { - return "", "", clues.Stack(errKnownSkippableCase).With("site_id", id) + return "", "", clues.Stack(errKnownSkippableCase).With("site_id", clues.Hide(id)) } return url, id, nil diff --git a/src/internal/connector/onedrive/drive.go b/src/internal/connector/onedrive/drive.go index f443cb8c7..117e8f288 100644 --- a/src/internal/connector/onedrive/drive.go +++ b/src/internal/connector/onedrive/drive.go @@ -303,7 +303,7 @@ func GetAllFolders( name = ptr.Val(d.GetName()) ) - ictx := clues.Add(ctx, "drive_id", id, "drive_name", name) // TODO: pii + ictx := clues.Add(ctx, "drive_id", id, "drive_name", clues.Hide(name)) collector := func( _ context.Context, _, _ string, diff --git a/src/internal/connector/onedrive/restore.go b/src/internal/connector/onedrive/restore.go index 3505f6c2b..765bc38b9 100644 --- a/src/internal/connector/onedrive/restore.go +++ b/src/internal/connector/onedrive/restore.go @@ -75,9 +75,9 @@ func RestoreCollections( err error ictx = clues.Add( ctx, - "resource_owner", dc.FullPath().ResourceOwner(), // TODO: pii + "resource_owner", clues.Hide(dc.FullPath().ResourceOwner()), "category", dc.FullPath().Category(), - "path", dc.FullPath()) // TODO: pii + "path", dc.FullPath()) // TODO: pii, path needs concealer compliance ) metrics, folderMetas, err = RestoreCollection( diff --git a/src/internal/connector/sharepoint/restore.go b/src/internal/connector/sharepoint/restore.go index afa7a2dd8..1f77b7b7a 100644 --- a/src/internal/connector/sharepoint/restore.go +++ b/src/internal/connector/sharepoint/restore.go @@ -61,8 +61,8 @@ func RestoreCollections( metrics support.CollectionMetrics ictx = clues.Add(ctx, "category", category, - "destination", dest.ContainerName, // TODO: pii - "resource_owner", dc.FullPath().ResourceOwner()) // TODO: pii + "destination", clues.Hide(dest.ContainerName), + "resource_owner", clues.Hide(dc.FullPath().ResourceOwner())) ) switch dc.FullPath().Category() { diff --git a/src/internal/operations/backup.go b/src/internal/operations/backup.go index 340d77fbd..08de9678a 100644 --- a/src/internal/operations/backup.go +++ b/src/internal/operations/backup.go @@ -144,8 +144,8 @@ func (op *BackupOperation) Run(ctx context.Context) (err error) { ctx = clues.Add( ctx, - "tenant_id", op.account.ID(), // TODO: pii - "resource_owner", op.ResourceOwner, // TODO: pii + "tenant_id", clues.Hide(op.account.ID()), + "resource_owner", clues.Hide(op.ResourceOwner), "backup_id", op.Results.BackupID, "service", op.Selectors.Service, "incremental", op.incremental) @@ -549,7 +549,7 @@ func mergeDetails( if err != nil { return clues.New("parsing base item info path"). WithClues(mctx). - With("repo_ref", entry.RepoRef) // todo: pii + With("repo_ref", entry.RepoRef) // todo: pii, path needs concealer compliance } // Although this base has an entry it may not be the most recent. Check diff --git a/src/internal/operations/restore.go b/src/internal/operations/restore.go index 1220cfe66..f11b3e56b 100644 --- a/src/internal/operations/restore.go +++ b/src/internal/operations/restore.go @@ -134,10 +134,10 @@ func (op *RestoreOperation) Run(ctx context.Context) (restoreDetails *details.De ctx = clues.Add( ctx, - "tenant_id", op.account.ID(), // TODO: pii + "tenant_id", clues.Hide(op.account.ID()), "backup_id", op.BackupID, "service", op.Selectors.Service, - "destination_container", op.Destination.ContainerName) + "destination_container", clues.Hide(op.Destination.ContainerName)) // ----- // Execution diff --git a/src/pkg/logger/example_logger_test.go b/src/pkg/logger/example_logger_test.go index 6241f4b99..13b1ee417 100644 --- a/src/pkg/logger/example_logger_test.go +++ b/src/pkg/logger/example_logger_test.go @@ -5,7 +5,9 @@ import ( "github.com/alcionai/clues" + "github.com/alcionai/corso/src/internal/connector" "github.com/alcionai/corso/src/pkg/logger" + "github.com/alcionai/corso/src/pkg/path" ) // --------------------------------------------------------------------------- @@ -18,7 +20,10 @@ const ( itemID = "item_id" ) -var err error +var ( + err error + itemPath, _ = path.Build("tid", "own", path.ExchangeService, path.ContactsCategory, false, "foo") +) // --------------------------------------------------------------------------- // examples @@ -129,6 +134,36 @@ func Example_logger_clues_standards() { // in the expected format. logger.CtxErr(ctx, err).Error("getting item") - // TODO(keepers): PII // 3. Protect pii in logs. + // When it comes to protecting sensitive information, we only want + // to hand loggers (and, by extension, clues errors) using one of + // three approaches to securing values. + // + // First: plain, unhidden data. This can only be logged if we are + // absolutely assured that this data does not expose sensitive + // information for a user. Eg: internal ids and enums are fine to + // log in plain text. Everything else must be considered wisely. + // + // Second: manually concealed values. Strings containing sensitive + // info, and structs from external pacakges containing sensitive info, + // can be logged by manually wrapping them with a clues.Hide() call. + // Ex: clues.Hide(userName). This will hash the value according to + // the user's hash algorithm configuration. + // + // Third: structs that comply with clues.Concealer. The Concealer + // interface requires a struct to comply with Conceal() (for cases + // where the struct is handed to a clues aggregator directly), and + // fmt's Format(state, verb), where the assumption is the standard + // format writer will be replaced with a Conceal() call (for cases + // where the struct is handed to some non-compliant formatter/printer). + // + // preferred + log.With( + // internal type, safe to log plainly + "resource_type", connector.Users, + // string containing sensitive info, wrap with Hide() + "user_name", clues.Hide("your_user_name@microsoft.example"), + // a concealer-compliant struct, safe to add plainly + "storage_path", itemPath, + ) } diff --git a/src/pkg/repository/repository.go b/src/pkg/repository/repository.go index 1e340f9dc..1a9423677 100644 --- a/src/pkg/repository/repository.go +++ b/src/pkg/repository/repository.go @@ -105,7 +105,7 @@ func Initialize( ctx = clues.Add( ctx, "acct_provider", acct.Provider.String(), - "acct_id", acct.ID(), // TODO: pii + "acct_id", clues.Hide(acct.ID()), "storage_provider", s.Provider.String()) defer func() { @@ -179,7 +179,7 @@ func Connect( ctx = clues.Add( ctx, "acct_provider", acct.Provider.String(), - "acct_id", acct.ID(), // TODO: pii + "acct_id", clues.Hide(acct.ID()), "storage_provider", s.Provider.String()) defer func() { diff --git a/src/pkg/services/m365/m365.go b/src/pkg/services/m365/m365.go index e078152cf..4f620ce67 100644 --- a/src/pkg/services/m365/m365.go +++ b/src/pkg/services/m365/m365.go @@ -6,6 +6,7 @@ import ( "github.com/alcionai/clues" "github.com/microsoftgraph/msgraph-sdk-go/models" + "github.com/alcionai/corso/src/internal/common/ptr" "github.com/alcionai/corso/src/internal/connector" "github.com/alcionai/corso/src/internal/connector/discovery" "github.com/alcionai/corso/src/internal/connector/graph" @@ -137,7 +138,7 @@ func SiteIDs(ctx context.Context, acct account.Account, errs *fault.Bus) ([]stri func parseUser(item models.Userable) (*User, error) { if item.GetUserPrincipalName() == nil { return nil, clues.New("user missing principal name"). - With("user_id", *item.GetId()) // TODO: pii + With("user_id", clues.Hide(ptr.Val(item.GetId()))) } u := &User{PrincipalName: *item.GetUserPrincipalName(), ID: *item.GetId()}