fix: wif and UI scale#10
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR updates the frontend layout to constrain content width for better UI scaling and improves cloud/matcher behavior by supporting GCP Workload Identity Federation credentials and making indicator matching case-insensitive.
Changes:
- Constrain routed page content to a centered max width container while keeping padding.
- Detect GCP credentials JSON type (service_account, external_account/WIF, authorized_user) before building client options.
- Make Elastic/Datadog indicator matching case-insensitive and add regression tests.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| web/frontend/src/routes/+layout.svelte | Wrapes page content in a centered, max-width container to improve UI scaling. |
| pack/tags.go | Adds Azure storage data-plane resources to the “no tags” allowlist. |
| pack/gcp/gcp.go | Adds credential-type detection to support WIF/external_account credentials. |
| internal/matchers/elastic/elastic.go | Makes indicator matching case-insensitive. |
| internal/matchers/elastic/elastic_test.go | Adds regression test for case-insensitive matching. |
| internal/matchers/datadog/datadog.go | Makes indicator matching case-insensitive. |
| internal/matchers/datadog/datadog_test.go | Adds regression test for case-insensitive matching. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
368
to
+370
| func alertMatchesIndicators(alert ElasticSecurityDetectionAlert, indicators []string) bool { | ||
| alertBytes, _ := json.Marshal(alert.Source) | ||
| alertString := string(alertBytes) | ||
| alertString := strings.ToLower(string(alertBytes)) |
Comment on lines
189
to
+191
| func (m *DatadogAlertGeneratedAssertion) signalMatchesExecution(signal datadogV2.SecurityMonitoringSignal, indicators []string, logger *logrus.Entry) bool { | ||
| buf, _ := json.Marshal(signal.Attributes.Custom) | ||
| rawSignal := string(buf) | ||
| rawSignal := strings.ToLower(string(buf)) |
Comment on lines
372
to
+373
| for _, indicator := range indicators { | ||
| if strings.Contains(alertString, indicator) { | ||
| if strings.Contains(alertString, strings.ToLower(indicator)) { |
Comment on lines
193
to
+194
| for _, indicator := range indicators { | ||
| if strings.Contains(rawSignal, indicator) { | ||
| if strings.Contains(rawSignal, strings.ToLower(indicator)) { |
Comment on lines
+150
to
+154
| // credentialsType inspects the "type" field of a GCP credentials JSON document | ||
| // and returns the matching CredentialsType. It supports service_account keys, | ||
| // external_account (Workload Identity Federation) configs, and authorized_user | ||
| // credentials. The credentials are produced by the simrun credential resolver, | ||
| // not an untrusted source. |
Comment on lines
+111
to
+112
| <main class="flex-1 min-w-0"> | ||
| <div class="mx-auto w-full max-w-[1536px] p-6"> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes credential handling for GCP Workload Identity Federation, makes alert/signal indicator matching case-insensitive, expands the Azure no-tags resource list, and constrains main content width for better UI scaling on wide displays.
Bug Fixes
GOOGLE_CREDENTIALSwas always parsed as aservice_accountkey, which broke Workload Identity Federation. Now acredentialsTypehelper inspects the JSONtypefield and supportsservice_account,external_account(WIF), andauthorized_user, failing loudly on missing/unsupported types.strings.Contains, causing missed matches when providers (e.g. Azure) emit custom fields in a different case than the indicator. Both sides are now lower-cased before comparison.azurerm_storage_container,azurerm_storage_blob,azurerm_storage_share*,azurerm_storage_queue,azurerm_storage_table*,azurerm_storage_data_lake_gen2_*) toazureNoTagsResources, since these are scoped to a storage account and reject tags.UI
max-w-[1536px]so layouts no longer stretch edge-to-edge on ultra-wide screens.Tests
TestSignalMatchesExecutionCaseInsensitive(Datadog) andTestAlertMatchesIndicatorsCaseInsensitive(Elastic), both of which fail against the old case-sensitive implementation and verify a genuinely absent value still doesn't match.