Skip to content

Commit 0385b3b

Browse files
committed
fix(review): support clearing chat_webhook_url via empty string
- Remove dsl.Format(dsl.FormatURI) from ChatWebhookURLAttribute so that an empty string passes Goa validation (FormatURI rejected "") - Change httpsURLPattern to ^$|^https://... to allow the empty-string clear signal while still requiring HTTPS for actual URLs - Update preserve/clear guard in UpdateSettings: nil/absent preserves the existing URL (safe for GET→PUT round-trips); "" explicitly clears it (sets stored value to nil); a non-empty URL replaces it - Add comments on all three sanitization sites (create indexer, update indexer, NATS event old/new snapshots) - Add TestCommitteeWriterOrchestrator_UpdateSettings_ChatWebhookURLSemantics covering omit-preserves, empty-string-clears, replace, and nil-stored cases LFXV2-3094 Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Andres Tobon <andrest2455@gmail.com>
1 parent 70ce059 commit 0385b3b

11 files changed

Lines changed: 129 additions & 49 deletions

File tree

cmd/committee-api/design/type.go

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -208,9 +208,11 @@ func DescriptionAttribute() {
208208
// would otherwise pass dsl.FormatURI as syntactically valid URIs.
209209
const urlPattern = `^https?://[^\s/$.?#][^\s]*$`
210210

211-
// httpsURLPattern validates an HTTPS-only URL. Used for fields that carry
212-
// bearer credentials (e.g. webhook URLs) where plaintext HTTP is unsafe.
213-
const httpsURLPattern = `^https://[^\s/$.?#][^\s]*$`
211+
// httpsURLPattern validates an HTTPS-only URL or an empty string.
212+
// Used for write-only credential fields (e.g. webhook URLs) where:
213+
// - a non-empty value must be a valid HTTPS URL (plaintext HTTP is rejected)
214+
// - an empty string is the explicit clear signal (see ChatWebhookURLAttribute)
215+
const httpsURLPattern = `^$|^https://[^\s/$.?#][^\s]*$`
214216

215217
// WebsiteAttribute is the DSL attribute for committee website.
216218
func WebsiteAttribute() {
@@ -1033,9 +1035,10 @@ func ChatChannelAttribute() {
10331035
}
10341036

10351037
// ChatWebhookURLAttribute is the DSL attribute for the committee's Slack Incoming Webhook URL.
1038+
// This field is write-only: accepted on create and PUT settings, never returned from GET.
1039+
// Update semantics: omit or send null to preserve the stored URL; send "" to clear it.
10361040
func ChatWebhookURLAttribute() {
1037-
dsl.Attribute("chat_webhook_url", dsl.String, "Slack Incoming Webhook URL for sharing content to a Slack channel", func() {
1038-
dsl.Format(dsl.FormatURI)
1041+
dsl.Attribute("chat_webhook_url", dsl.String, "Slack Incoming Webhook URL for sharing content to a Slack channel. Write-only: never returned from GET. Send empty string to clear a previously stored value; omit the field (or send null) to preserve the existing value.", func() {
10391042
dsl.Pattern(httpsURLPattern)
10401043
dsl.MaxLength(500)
10411044
dsl.Example("https://hooks.slack.example.org/services/TXXXXXXXX/BXXXXXXXX/placeholder")

gen/committee_service/service.go

Lines changed: 6 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

gen/http/committee_service/client/cli.go

Lines changed: 2 additions & 8 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

gen/http/committee_service/client/types.go

Lines changed: 6 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

gen/http/committee_service/server/types.go

Lines changed: 8 additions & 10 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

gen/http/openapi.json

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

gen/http/openapi.yaml

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4217,10 +4217,9 @@ definitions:
42174217
maxLength: 500
42184218
chat_webhook_url:
42194219
type: string
4220-
description: Slack Incoming Webhook URL for sharing content to a Slack channel
4220+
description: 'Slack Incoming Webhook URL for sharing content to a Slack channel. Write-only: never returned from GET. Send empty string to clear a previously stored value; omit the field (or send null) to preserve the existing value.'
42214221
example: https://hooks.slack.example.org/services/TXXXXXXXX/BXXXXXXXX/placeholder
4222-
format: uri
4223-
pattern: ^https://[^\s/$.?#][^\s]*$
4222+
pattern: ^$|^https://[^\s/$.?#][^\s]*$
42244223
maxLength: 500
42254224
deliverables:
42264225
type: array
@@ -4978,10 +4977,9 @@ definitions:
49784977
example: false
49794978
chat_webhook_url:
49804979
type: string
4981-
description: Slack Incoming Webhook URL for sharing content to a Slack channel
4980+
description: 'Slack Incoming Webhook URL for sharing content to a Slack channel. Write-only: never returned from GET. Send empty string to clear a previously stored value; omit the field (or send null) to preserve the existing value.'
49824981
example: https://hooks.slack.example.org/services/TXXXXXXXX/BXXXXXXXX/placeholder
4983-
format: uri
4984-
pattern: ^https://[^\s/$.?#][^\s]*$
4982+
pattern: ^$|^https://[^\s/$.?#][^\s]*$
49854983
maxLength: 500
49864984
last_reviewed_at:
49874985
type: string

gen/http/openapi3.json

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

gen/http/openapi3.yaml

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6890,10 +6890,9 @@ components:
68906890
maxLength: 500
68916891
chat_webhook_url:
68926892
type: string
6893-
description: Slack Incoming Webhook URL for sharing content to a Slack channel
6893+
description: 'Slack Incoming Webhook URL for sharing content to a Slack channel. Write-only: never returned from GET. Send empty string to clear a previously stored value; omit the field (or send null) to preserve the existing value.'
68946894
example: https://hooks.slack.example.org/services/TXXXXXXXX/BXXXXXXXX/placeholder
6895-
format: uri
6896-
pattern: ^https://[^\s/$.?#][^\s]*$
6895+
pattern: ^$|^https://[^\s/$.?#][^\s]*$
68976896
maxLength: 500
68986897
deliverables:
68996898
type: array
@@ -8255,10 +8254,9 @@ components:
82558254
example: false
82568255
chat_webhook_url:
82578256
type: string
8258-
description: Slack Incoming Webhook URL for sharing content to a Slack channel
8257+
description: 'Slack Incoming Webhook URL for sharing content to a Slack channel. Write-only: never returned from GET. Send empty string to clear a previously stored value; omit the field (or send null) to preserve the existing value.'
82598258
example: https://hooks.slack.example.org/services/TXXXXXXXX/BXXXXXXXX/placeholder
8260-
format: uri
8261-
pattern: ^https://[^\s/$.?#][^\s]*$
8259+
pattern: ^$|^https://[^\s/$.?#][^\s]*$
82628260
maxLength: 500
82638261
last_reviewed_at:
82648262
type: string

internal/service/committee_writer.go

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -526,7 +526,7 @@ func (uc *committeeWriterOrchestrator) Create(ctx context.Context, committee *mo
526526

527527
if committee.CommitteeSettings != nil {
528528
indexSettings := *committee.CommitteeSettings
529-
indexSettings.ChatWebhookURL = nil
529+
indexSettings.ChatWebhookURL = nil // bearer credential — must not enter the search index
530530
settingsMsg, errBuildSettingsMsg := uc.buildIndexerMessage(ctx, model.ActionCreated, &indexSettings, committee.Tags())
531531
if errBuildSettingsMsg != nil {
532532
return nil, errs.NewUnexpected("failed to build indexer message", errBuildSettingsMsg)
@@ -858,12 +858,13 @@ func (uc *committeeWriterOrchestrator) UpdateSettings(ctx context.Context, setti
858858
settings.Auditors = existingSettings.Auditors
859859
}
860860

861-
// Preserve the stored webhook URL when the caller omits the field.
862-
// Because chat_webhook_url is write-only (not returned from GET), a
863-
// GET→PUT round-trip will always arrive with nil here; overwriting on
864-
// every omission would silently wipe the credential on any settings save.
861+
// nil (absent/null) → preserve existing; "" → explicit clear; non-empty URL → replace.
862+
// Because chat_webhook_url is write-only (not returned from GET), a GET→PUT
863+
// round-trip always sends nil — preserve prevents silently wiping the credential.
865864
if settings.ChatWebhookURL == nil {
866865
settings.ChatWebhookURL = existingSettings.ChatWebhookURL
866+
} else if *settings.ChatWebhookURL == "" {
867+
settings.ChatWebhookURL = nil
867868
}
868869

869870
// Step 3: Update the committee settings in storage
@@ -894,7 +895,7 @@ func (uc *committeeWriterOrchestrator) UpdateSettings(ctx context.Context, setti
894895
committee := &model.Committee{CommitteeBase: *committeeBase, CommitteeSettings: settings}
895896
// Build and publish indexer message
896897
indexSettings := *settings
897-
indexSettings.ChatWebhookURL = nil
898+
indexSettings.ChatWebhookURL = nil // bearer credential — must not enter the search index
898899
messageIndexer, errBuildIndexerMessage := uc.buildIndexerMessage(ctx, model.ActionUpdated, &indexSettings, committee.Tags())
899900
if errBuildIndexerMessage != nil {
900901
slog.ErrorContext(ctx, "failed to build indexer message",
@@ -927,9 +928,9 @@ func (uc *committeeWriterOrchestrator) UpdateSettings(ctx context.Context, setti
927928
// detect newly added Writers/Auditors and send notification emails.
928929
updatedBy, _ := ctx.Value(constants.PrincipalContextID).(string)
929930
sanitizedOldSettings := *existingSettings
930-
sanitizedOldSettings.ChatWebhookURL = nil
931+
sanitizedOldSettings.ChatWebhookURL = nil // bearer credential — must not enter NATS events
931932
sanitizedNewSettings := *settings
932-
sanitizedNewSettings.ChatWebhookURL = nil
933+
sanitizedNewSettings.ChatWebhookURL = nil // bearer credential — must not enter NATS events
933934
settingsEvent, errBuildEvent := (&model.CommitteeEvent{}).Build(ctx, model.ResourceCommitteeSettings, model.ActionUpdated, &model.CommitteeSettingsUpdateEventData{
934935
CommitteeUID: settings.UID,
935936
OldSettings: &sanitizedOldSettings,

0 commit comments

Comments
 (0)