Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Update sensitive metadata allowlist to filter on keys#62830

Merged
akalia25 merged 19 commits into
mainfrom
ak/update-telemetry-sensitive-metadata-allowlist
May 22, 2024
Merged

Update sensitive metadata allowlist to filter on keys#62830
akalia25 merged 19 commits into
mainfrom
ak/update-telemetry-sensitive-metadata-allowlist

Conversation

@akalia25

@akalia25 akalia25 commented May 21, 2024

Copy link
Copy Markdown
Contributor

This pull request introduces a mechanism to selectively include specific privateMetadata keys to be allowlisted and sent for all users. Previously, allowlisting only worked to allowlist all keys on the event to be sent. This gives us greater flexibility/safety on what fields we'll be sending on events that have an allowList.

Benefits

  • Enables selective logging of privateMetadata keys that are deemed safe and valuable for analysis.
  • Maintains privacy and security by excluding potentially sensitive metadata fields not present in the allowed list, but are in the event.

co-authored with: @bobheadxi

Test plan

  • Added unit tests to verify expected behavior

Rik and others added 10 commits May 15, 2024 13:53
* Fix bedrock URL encoding to mimic AWS CLI

* Update changelog
Allow a namespace to be configured, defaulting to all namespaces.
Without this setting, if an admin deploys the appliance with
namespace-scoped RBAC, it would throw errors due to not being able to
watch ConfigMaps in all namespaces.
Without this, this error won't be logged to Sentry, resulting in us missing it unless we check GCP

## Test plan

Discussed with @jac
Fix global header navigation layers
Now that #62644 (CORE-23) is rolled out, this import block is no longer needed (and may even be disruptive when provisioning new rollout pipelines). The change was rolled out in:

- sourcegraph/managed-services#1416
- sourcegraph/managed-services#1417
- sourcegraph/managed-services#1403

## Test plan

n/a
This PR creates a new GraphQL integration test file focused on symbol search.
It exercises the same searches the web client uses for code navigation.

In a follow-up, we will add cases for older commits and enable Rockskip.
@cla-bot cla-bot Bot added the cla-signed label May 21, 2024
@akalia25 akalia25 marked this pull request as ready for review May 21, 2024 20:08
@akalia25 akalia25 requested a review from bobheadxi May 21, 2024 21:00
@bobheadxi bobheadxi requested a review from jac May 22, 2024 03:28
Comment thread internal/telemetry/sensitivemetadataallowlist/redact.go Outdated
Comment thread internal/telemetry/sensitivemetadataallowlist/redact.go Outdated
Comment thread internal/telemetry/sensitivemetadataallowlist/sensitivemetadataallowlist.go Outdated
@bobheadxi

Copy link
Copy Markdown
Member

For context, Aditya and I paired on this over a call; I'm aligned with overall direction, but let's make sure everything is sufficiently documented :)

Comment thread internal/telemetry/sensitivemetadataallowlist/redact_test.go
Comment thread internal/telemetry/sensitivemetadataallowlist/sensitivemetadataallowlist.go Outdated
Comment thread internal/telemetry/teestore/teestore_test.go Outdated

@bobheadxi bobheadxi left a comment

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.

Nice work!

@akalia25 akalia25 merged commit 9123e60 into main May 22, 2024
@akalia25 akalia25 deleted the ak/update-telemetry-sensitive-metadata-allowlist branch May 22, 2024 20:29
bobheadxi added a commit that referenced this pull request May 28, 2024
Fixes a regression from #62830 that caused us to remove all sensitive metadata in dotcom mode, even though we generally collect all sensitive metadata in dotcom. This also fixes export of actually-allowlisted private metadata from custom instances.

tl;dr we accidentally started removing all private metadata, all the time

## Test plan

New unit test covering the redact mode that was evaluated
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants