tien238lnd opened a new issue, #43574:
URL: https://github.com/apache/superset/issues/43574
### Bug description
A dashboard layout whose `children` array names a node id that is not
defined in that layout is accepted by `PUT /api/v1/dashboard/{id}` and stored.
From then on, **every** update to that dashboard fails with a 500 — including
the update that would repair the layout. The dashboard becomes unwritable
through the REST API.
Three things combine to make this hard to recover from and hard to diagnose:
1. The request that stores the broken layout returns **200**, so a client
gets no signal that anything is wrong.
2. The failure surfaces on the **next** request, not the one that caused it.
3. `UpdateDashboardCommand.process_tab_diff` reads the layout from the
**database**, not from the request body, so a corrective `PUT` carrying a valid
`position_json` 500s before it is ever applied.
The only way out is `export` → edit the YAML → `import` with overwrite.
`position_json` is validated for JSON parseability only.
`DashboardPutSchema.position_json` (`superset/dashboards/schemas.py`) uses
`validate_json`, which just calls `json.loads` and raises on `JSONDecodeError`.
Nothing checks that the layout is structurally coherent.
### Reproduction steps
1. `PUT /api/v1/dashboard/{id}` with this `position_json` (`TABS-1`
references `TAB-2`, which is never defined):
```json
{
"DASHBOARD_VERSION_KEY": "v2",
"ROOT_ID": {"type": "ROOT", "id": "ROOT_ID", "children": ["GRID_ID"]},
"GRID_ID": {"type": "GRID", "id": "GRID_ID", "children": ["TABS-1"],
"parents": ["ROOT_ID"]},
"TABS-1": {"type": "TABS", "id": "TABS-1", "children": ["TAB-1",
"TAB-2"], "parents": ["ROOT_ID", "GRID_ID"], "meta": {}},
"TAB-1": {"type": "TAB", "id": "TAB-1", "children": [], "parents":
["ROOT_ID", "GRID_ID", "TABS-1"], "meta": {"text": "First"}}
}
```
→ **200 OK**. The layout is stored.
2. `GET /api/v1/dashboard/{id}/tabs` → **500**
3. `PUT /api/v1/dashboard/{id}` with any body at all, e.g. `{"published":
true}` → **500**
4. There is no payload that fixes it, because step 3 reads the stored layout
before applying anything.
Two other layouts reach the same dead end:
- A `TAB` node with no `meta.text`.
- A non-empty `position_json` with no `ROOT_ID` key, e.g. `{"foo": 1}`.
## Expected results
Either the request in step 1 is rejected, or `Dashboard.tabs` tolerates a
layout it cannot fully resolve. A dashboard should not be able to reach a state
where a write is accepted and then permanently locks out every subsequent
write, the repair included.
## Actual results
`Dashboard.tabs` (`superset/models/dashboard.py`) raises `KeyError` at three
places, all reached on data that is already in the database:
- `get_node` — `return self.position[node_id]`, for a `children` entry that
does not resolve
- `node["meta"]["text"]` — for a `TAB` node with no `meta`
- `root = get_node("ROOT_ID")` — for a non-empty layout with no `ROOT_ID`
`process_tab_diff` calls `self._model.tabs` unconditionally, before it
checks whether the payload even carries a `position_json`, so the `KeyError`
propagates out of every `PUT`.
Tracebacks, from running each layout above against `Dashboard.tabs` on
`master` (`e7ca8b8`):
```
======== A: a children entry names a node that is not defined in the layout
========
Traceback (most recent call last):
File "<string>", line 31, in <module>
File "superset/models/dashboard.py", line 424, in tabs
build_tab_tree(node, children)
File "superset/models/dashboard.py", line 401, in build_tab_tree
child = get_node(child_id)
^^^^^^^^^^^^^^^^^^
File "superset/models/dashboard.py", line 389, in get_node
return self.position[node_id]
~~~~~~~~~~~~~^^^^^^^^^
KeyError: 'TAB-2'
======== B: a TAB node carries no meta.text ========
Traceback (most recent call last):
File "<string>", line 31, in <module>
File "superset/models/dashboard.py", line 424, in tabs
build_tab_tree(node, children)
File "superset/models/dashboard.py", line 413, in build_tab_tree
node["title"] = node["meta"]["text"]
~~~~^^^^^^^^
KeyError: 'meta'
======== C: a non-empty layout with no ROOT_ID ========
Traceback (most recent call last):
File "<string>", line 31, in <module>
File "superset/models/dashboard.py", line 417, in tabs
root = get_node("ROOT_ID")
^^^^^^^^^^^^^^^^^^^
File "superset/models/dashboard.py", line 389, in get_node
return self.position[node_id]
~~~~~~~~~~~~~^^^^^^^^^
KeyError: 'ROOT_ID'
```
### Screenshots/recordings
Not applicable — this is a backend failure, reproducible entirely through
the REST API.
### Superset version
master / latest-dev
### Python version
3.11
### Node version
16
### Browser
Chrome
### Additional context
**The error shown in the UI is misleading.** Toggling publish on an affected
dashboard surfaces `You do not have permissions to edit this dashboard.` This
is not a permissions problem. `savePublished` in
`superset-frontend/src/dashboard/actions/dashboardState.ts:279` ends in a bare
`.catch(() => ...)` that raises that one toast for any failed `PUT`, a 500
included. That string occurs in exactly one place under
`superset-frontend/src`, so anyone hitting this is likely to spend time on
roles and ownership before finding the real cause.
**Possible follow-up.** Validating `position_json` structurally in
`DashboardPutSchema` would stop the broken layout being stored in the first
place. It is deliberately out of scope here: payloads that are accepted today
would start returning 400, which is a breaking change for any client scripting
dashboards, and doing it properly means formally specifying the `position_json`
invariants — something Superset has never specified. That belongs in its own
discussion.
### Checklist
- [x] I have searched Superset docs and Slack and didn't find a solution to
my problem.
- [x] I have searched the GitHub issue tracker and didn't find a similar bug
report.
- [x] I have checked Superset's logs for errors and if I found a relevant
Python stacktrace, I included it here as text in the "additional context"
section.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
