Skip to content

fix(api): reject invalid service edit payloads and announce every edit - #943

Merged
JosephKav merged 3 commits into
masterfrom
940-bug-put-apiv1serviceconfig-with-no-id-in-the-payload-destroys-the-service-and-corrupts-the-order-list
Jul 28, 2026
Merged

fix(api): reject invalid service edit payloads and announce every edit#943
JosephKav merged 3 commits into
masterfrom
940-bug-put-apiv1serviceconfig-with-no-id-in-the-payload-destroys-the-service-and-corrupts-the-order-list

Conversation

@JosephKav

Copy link
Copy Markdown
Collaborator
  • No id in the payload - accepted with 200 OK, replacing the service
    with an empty-ID one and corrupting the order. Now a 400.
  • Notify/webhook entries with a missing or duplicate name - stored under
    "", or silently collapsed into the last one. Now a 400, naming the index.
  • Edits that change nothing the dashboard shows - never broadcast, so other
    clients kept a stale config for the 5 minute staleTime and could silently
    revert the edit on their next save. Now always announced.

Fixes #940

A PUT to /api/v1/service/config or POST to /api/v1/service/new without
an `id` in the payload was incorrectly accepted, while still returning
`200 OK`. An edit replaced the target service with one whose ID was an
empty string, leaving an invalid entry in its place in the service
order. A create added one under that same empty ID.

Both endpoints now reject requests with no `id`, with `400 Bad Request`,
leaving the service and service order unchanged. `AddService` and
`RenameService` also reject empty IDs.

Existing configurations containing a service with an empty ID are left
unchanged, but with a warning log identifying the issue, as they require
manual repair.

Also document that the endpoint body replaces an existing service rather
than merging into it.
The service edit payload takes an array for notify and webhook entries
keyed by their `name`, so an entry sent without one was stored under the
empty string, and two entries sharing a name silently collapsed into the
last one. Both were accepted with `200 OK`.

Each is now rejected with `400 Bad Request`, naming the offending
index (matching what the web UI already enforces).
@JosephKav JosephKav added bug Something isn't working go Pull requests that update Go code labels Jul 26, 2026
announceEdit skipped the broadcast when RemoveUnchanged left the summary
empty, so edits to fields absent from it reached no other client. Those
clients kept the service config cached for the staleTime, so reopening
the edit modal repopulated the form from stale data and saving silently
reverted the edit.

Also fixes the rename branch merging cached data over the new ID.
@JosephKav
JosephKav force-pushed the 940-bug-put-apiv1serviceconfig-with-no-id-in-the-payload-destroys-the-service-and-corrupts-the-order-list branch from 75c1a75 to 256f51c Compare July 26, 2026 04:33
@codecov

codecov Bot commented Jul 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.71%. Comparing base (43104b9) to head (256f51c).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #943      +/-   ##
==========================================
+ Coverage   99.71%   99.71%   +0.01%     
==========================================
  Files         204      204              
  Lines       12946    12988      +42     
==========================================
+ Hits        12908    12950      +42     
  Misses         33       33              
  Partials        5        5              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

Copy link
Copy Markdown

✅ Playwright Test Results

Count
✅ Passed 318
❌ Failed 0
🔄 Flaky 1
⏭️ Skipped 4

Duration: 1057.8s

View screenshots & full report

@JosephKav
JosephKav merged commit 227a79b into master Jul 28, 2026
8 checks passed
@JosephKav
JosephKav deleted the 940-bug-put-apiv1serviceconfig-with-no-id-in-the-payload-destroys-the-service-and-corrupts-the-order-list branch July 28, 2026 01:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working go Pull requests that update Go code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: PUT /api/v1/service/config with no 'id' in the payload destroys the service and corrupts the order list

2 participants