Skip to content

Commit a371c5b

Browse files
committed
fix(config): allow clearing a webhook auth header
Editing an outbound webhook and leaving the auth-header field blank preserves the stored header (intended), but there was no way to remove a header once set — blank always meant 'keep', and no deletion gesture existed. Treat a single '-' in the auth field as an explicit clear (Headers = null); blank still preserves. The edit-form placeholder now reads '(stored header preserved — enter - to clear)' so the gesture is discoverable.
1 parent cc112ea commit a371c5b

4 files changed

Lines changed: 31 additions & 4 deletions

File tree

openspec/changes/harden-config-tui-io-and-failloud/tasks.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ Cited file:line numbers are from the review doc and may drift as fixes land. -->
3535
- [x] 3.2 `ProviderManagerViewModel` (review:519) — write the fixed credential to disk only after the probe succeeds; a failed probe preserves the prior secret. Test: probe-fail leaves the stored secret unchanged. — `SubmitFixCredentials` no longer writes `secrets.json`/`netclaw.json` before probing; the write moved into a new `WriteFixedCredentials` helper called only from the `IsFixFlow` probe-**success** branch (matching the add flow's deferred `WriteProviderConfig`). A typo in the new API key/endpoint no longer clobbers the working credential with no rollback. New `FixCredentials_ProbeFailure_LeavesStoredSecretUnchanged` (secrets file byte-unchanged; bad key never written).
3636
- [x] 3.3 `SlackStepViewModel.BuildChannelAudiences` (review:299) — do not write an unresolved channel name as an ACL key; omit/flag it (or block) so it grants nothing. Test: unresolved channel → no inert name key in `ChannelAudiences`. — `ResolveChannelAudienceKey` (which returned the channel NAME when `LastChannelResolution` was null or the name was unresolved) is now `TryResolveChannelAudienceKey`: it yields a key only for the DM row (`"dm"`) or a resolved canonical channel ID, and `BuildChannelAudiences` omits any entry it can't resolve — so a dead, name-keyed ACL entry the Slack runtime can never match is never written. The health-check phase already warns about unresolved channels. New `ContributeConfig_OmitsUnresolvedChannelNameFromAudiences_KeepsResolvedById`.
3737
- [x] 3.4 `ChannelsConfigViewModel.ApplyAddChannelAsync` (review:582) — replace `Single()` with a safe predicate so a resolved id of `"dm"` with DMs enabled does not throw. Test: add a `"dm"`-resolving channel → no exception. — The row-focus lookup that positioned `_channelRowIndex` used `Single(row.Id == channelId)`; a resolved id of exactly `"dm"` with DMs enabled collided with the DM row (also `Id="dm"`) → `InvalidOperationException` crashing the add flow. Now `FirstOrDefault` over `!IsDirectMessage && !IsAction && Id==channelId` (matches only real channel rows), guarded against not-found. New `Add_channel_resolving_to_dm_with_dms_enabled_does_not_throw`.
38-
- [ ] 3.5 `TelemetryAlertingConfigViewModel` (review:289) — add an explicit gesture to clear a webhook auth header (blank-preserve still keeps it). Test: clear gesture removes the header; blank keeps it.
38+
- [x] 3.5 `TelemetryAlertingConfigViewModel` (review:289) — add an explicit gesture to clear a webhook auth header (blank-preserve still keeps it). Test: clear gesture removes the header; blank keeps it. — `SaveWebhookForm` now treats a single `-` in the auth field as an explicit clear (`target.Headers = null`); blank still preserves the stored header. The edit-form placeholder now reads `(stored header preserved — enter - to clear)` so the gesture is discoverable. New `Editing_a_webhook_clears_the_auth_header_with_the_dash_gesture`; the existing blank-preserve test still passes.
3939
- [ ] 3.6 `McpToolPermissionsViewModel.BuildAllowedServerList` (review:533) — operate on a copy, not the live in-memory profile object. Test: building the list does not mutate the source profile.
4040

4141
## 4. Verification & close

src/Netclaw.Cli.Tests/Tui/Config/TelemetryAlertingConfigViewModelTests.cs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,23 @@ public void Editing_a_webhook_replaces_the_auth_header_when_a_nonblank_header_is
161161
Assert.Equal("Bearer new", webhook.Headers?["Authorization"]);
162162
}
163163

164+
[Fact]
165+
public void Editing_a_webhook_clears_the_auth_header_with_the_dash_gesture()
166+
{
167+
File.WriteAllText(_paths.NetclawConfigPath,
168+
"{\"configVersion\":1,\"Notifications\":{\"Webhooks\":[{\"Name\":\"ops-alerts\",\"Url\":\"https://alerts.example.test/hook\",\"Headers\":{\"Authorization\":\"Bearer old\"},\"Format\":\"Generic\"}]}}");
169+
using var vm = new TelemetryAlertingConfigViewModel(_paths);
170+
171+
vm.BeginEditWebhook(0);
172+
Assert.True(vm.EditingHasPersistedAuthHeader.Value);
173+
// A blank field would preserve the header; "-" explicitly removes it.
174+
vm.WebhookAuthHeaderDraft.Value = "-";
175+
vm.ActivateSelected();
176+
177+
var webhook = Assert.Single(Bind<NotificationsConfig>("Notifications").Webhooks);
178+
Assert.Null(webhook.Headers);
179+
}
180+
164181
[Fact]
165182
public void Saving_a_webhook_without_a_url_is_rejected_before_persistence()
166183
{

src/Netclaw.Cli/Tui/Config/TelemetryAlertingConfigPage.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ private ILayoutNode BuildWebhookForm()
9898
{
9999
var format = ViewModel.DraftFormat;
100100
var authState = ViewModel.EditingHasPersistedAuthHeader.Value && string.IsNullOrWhiteSpace(ViewModel.WebhookAuthHeaderDraft.Value)
101-
? "(stored header preserved)"
101+
? "(stored header preserved — enter - to clear)"
102102
: string.IsNullOrWhiteSpace(ViewModel.WebhookAuthHeaderDraft.Value) ? "(optional)" : "(new header entered)";
103103

104104
var title = ViewModel.EditingHasPersistedAuthHeader.Value || !string.IsNullOrWhiteSpace(ViewModel.WebhookNameDraft.Value)

src/Netclaw.Cli/Tui/Config/TelemetryAlertingConfigViewModel.cs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -260,9 +260,13 @@ private void SaveWebhookForm()
260260
}
261261

262262
var authDraft = WebhookAuthHeaderDraft.Value.Trim();
263+
// A single "-" explicitly clears a persisted auth header; a blank field preserves it. Without
264+
// this gesture there is no way to remove a header once set (blank always means "keep").
265+
var clearAuth = authDraft == "-";
263266
string? headerName = null;
264267
string? headerValue = null;
265-
if (!string.IsNullOrWhiteSpace(authDraft)
268+
if (!clearAuth
269+
&& !string.IsNullOrWhiteSpace(authDraft)
266270
&& !TryParseHeader(authDraft, out headerName, out headerValue, out var headerError))
267271
{
268272
Status.Value = new ConfigStatusMessage(headerError, ConfigStatusTone.Error);
@@ -275,7 +279,7 @@ private void SaveWebhookForm()
275279
: WebhookNameDraft.Value.Trim();
276280

277281
var editing = _editingWebhookIndex;
278-
var newAuth = !string.IsNullOrWhiteSpace(authDraft);
282+
var newAuth = !clearAuth && !string.IsNullOrWhiteSpace(authDraft);
279283
var verb = editing is null ? "added" : "updated";
280284
var saved = PersistWebhooks(webhooks =>
281285
{
@@ -293,6 +297,12 @@ private void SaveWebhookForm()
293297
[headerName!] = headerValue!
294298
};
295299
}
300+
else if (clearAuth)
301+
{
302+
target.Headers = null;
303+
}
304+
305+
// Otherwise (blank, no "-"): leave target.Headers untouched so an unedited header is kept.
296306

297307
if (editing is null)
298308
webhooks.Add(target);

0 commit comments

Comments
 (0)