aws / aws/graph-explorer

Invalid color in an imported styling file permanently breaks the graph and schema views

Open
#2,114 0 comments 0 reactions 0 assignees View on GitHub
customization internal reliability
Dominant language
TypeScript
Stars
481
Forks
108
Avg merge
6d 8h
Merged PRs (30d)
5

Description

## Description

An unparseable color value in an imported styling file takes down the whole app, and it stays down across reloads.

`labelTextColorFor` (`packages/graph-explorer/src/core/StateProvider/graphElementStyleData.ts:64`) does:

```ts
new Color(labelColor || appDefaultEdgeStyle.labelColor);
```

The `||` guard only covers the empty string (the case tracked in #2110). Any *non-empty* value the `color` library can't parse — `"nope"`, `"#12"`, `"rgb("` — throws `Unable to parse color from string`. Confirmed: `new Color("nope")` throws.

## How the bad value gets in

Styling files are a shareable artifact users exchange, so their contents are effectively untrusted input. `core/styling/stylingParser.ts` validates the enum fields (`lineStyle`, border/arrow styles) but every color field is a bare `z.string().optional()`:

- edge entry — `labelColor`, `labelBorderColor`, `lineColor` (~lines 147-160)
- vertex entry — `color`, `borderColor` (~lines 131-140)

So a bogus color passes import validation and is persisted to IndexedDB under `user-edge-styles`.

## Why this is severe: it doesn't recover

1. `labelTextColorFor` is called from `edgeStyleData`, which both the canvas (`useRenderedEdges` in `core/StateProvider/renderedEntities.ts`) and the schema view (`useSchemaGraphEdges` in `modules/SchemaGraph/useSchemaGraphData.ts`) reach during render.
2. Neither view has a local error boundary. The throw unwinds to the app-level boundary in `DefaultLayout.tsx` (`FallbackComponent={AppErrorPage}`), replacing the entire UI with an error page whose only affordance is a reload.
3. Reload doesn't help. The value lives in IndexedDB, and `App.tsx` redirects `*` to `/graph-explorer`, so the landing route re-throws immediately.
4. The recovery path is broken too. Settings → Styles renders `components/LabelPreview.tsx`, which calls `labelTextColorFor` on the same value — so the page a user would visit to fix or reset the style also throws.

Net effect: the app is unusable until the user manually clears browser storage.

## Steps to Reproduce

1. Import a styling file containing `{"edges": {"SomeEdgeType": {"labelColor": "notacolor"}}}`.
2. Have at least one edge of that type on the canvas, or just open the schema view.
3. Observe the app-level error page. Reload — it comes straight back. Settings → Styles throws as well.

## Expected Behavior

An unparseable color is rejected at import with a clear per-field message, and a value already sitting in storage degrades gracefully to the default label color rather than taking down the app.

## Fix

Two parts, one at each boundary:

1. **Validate at import.** Replace `z.string()` with a color-validating refinement for every color field in `stylingParser.ts`, so a bad file becomes an import-time error with a field path — the parser already reports those well. Matches the "prefer Zod at boundaries" convention in `AGENTS.md`.
2. **Make `labelTextColorFor` total.** Catch the parse failure and fall back to the default label color, so a value already persisted in a user's IndexedDB can't brick the app.

## Also worth fixing here

`applyColor` in `packages/graph-explorer/src/core/icons/iconImageUrl.ts` concatenates the same unvalidated color into the SVG root's `style` attribute (`color:${color}`). Today the output is well-formed and only ever consumed as a `data:image/svg+xml` image (cytoscape `background-image`, and an ``), a context where nothing in it executes. But an arbitrary string is being spliced into a CSS declaration list, so a value with a `;` in it silently produces extra declarations. The comment there notes that #2105 ("tint everything") may inline this SVG as live DOM, at which point that splicing matters much more. Boundary color validation covers this case as well.

Note that `root.style.setProperty("color", color)` was tried and rejected: CSSOM normalises `#FF0000` to `rgb(255, 0, 0)`, changing the emitted data URI and risking silently dropping valid-but-exotic color values.

## Notes

Pre-existing on `main`. The `schema-view-style-perf` branch relocated this logic; it did not introduce it.

## Related Issues

- Related to #2110 — same missing validation in `stylingParser.ts`, but the blank-color case, which fails silently instead of throwing
- Related to #2105 — the "tint everything" work that would make the `applyColor` string splicing more consequential

> [!IMPORTANT]
> Internal only — this issue is maintained by the core team and is not accepting external contributions.

Contributor guide

Open the contributing guide

Research direction

Start with packages/graph-explorer/src/core/styling/stylingParser.ts and graphElementStyleData.ts, then inspect the callers in renderedEntities.ts, useSchemaGraphData.ts, and LabelPreview.tsx. Check the existing parser tests and run the relevant graph-explorer test suite. Done means invalid imported colors produce field-specific errors and persisted invalid values fall back without breaking graph, schema, or Styles views.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
frontend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.