Skip to content

Commit c900c95

Browse files
committed
fix(rich-markdown-editor): tighten read-only gate for uppercase entities and orphan ref-defs
Two silent-corruption cases the idempotency probe can't see: - The HTML-entity safe-list used a case-insensitive regex, so `&`/`<`/`>` were treated as the round-trippable canonical entities and let through as editable, but the serializer only round-trips the lowercase forms and mangles the uppercase ones. Make the safe-list case-sensitive. - An unused link/image reference definition (`[x]: url` with no `[x]` reference) is dropped entirely on serialize, a deletion the idempotency probe misses. Detect orphan definitions and open read-only; used definitions still inline losslessly and stay editable. GFM footnote definitions (`[^id]: …`) are excluded — they round-trip verbatim, so an unreferenced one stays editable rather than being flagged as an orphan.
1 parent b09e0c0 commit c900c95

2 files changed

Lines changed: 52 additions & 4 deletions

File tree

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/round-trip-safety.test.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,17 @@ describe('isRoundTripSafe', () => {
5555
expect(isRoundTripSafe('a b')).toBe(false)
5656
expect(isRoundTripSafe('a & b < c > d')).toBe(true)
5757
expect(isRoundTripSafe('AT&T and R&D')).toBe(true)
58+
expect(isRoundTripSafe('a & b')).toBe(false)
59+
expect(isRoundTripSafe('a < b > c')).toBe(false)
60+
})
61+
62+
it('rejects an orphan reference definition (serializer drops it) but allows used ones', () => {
63+
expect(isRoundTripSafe('Some text.\n\n[unused]: https://example.com "title"')).toBe(false)
64+
expect(isRoundTripSafe('[a]: u1\n[b]: u2\n\nuse only [a]')).toBe(false)
65+
expect(isRoundTripSafe('See [x][1].\n\n[1]: https://example.com "T"')).toBe(true)
66+
expect(isRoundTripSafe('A [shortcut] ref.\n\n[shortcut]: https://example.com')).toBe(true)
67+
expect(isRoundTripSafe('Case [Foo] insensitive.\n\n[foo]: https://example.com')).toBe(true)
68+
expect(isRoundTripSafe('A note.\n\n[^x]: the footnote body')).toBe(true)
5869
})
5970

6071
it('does not flag HTML/comments/entities inside tilde or nested code fences', () => {

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/round-trip-safety.ts

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,16 @@ const PROBE_SIZE_LIMIT = 256 * 1024
2424
* flattens `one<br>two` to `one two`. Matched on a table-shaped line (≥2 pipes) containing a `<br>`.
2525
* - **Hard break inside a heading** (trailing two spaces or a backslash) — the serializer splits
2626
* the heading, ejecting the second line into a separate paragraph.
27-
* - **HTML entity** other than `&amp;`/`&lt;`/`&gt;` (e.g. `&copy;`, `&#39;`, `&nbsp;`) — the
28-
* serializer escapes the `&`, turning the rendered character into literal entity source. A bare
29-
* `&` with no `;` is left alone (it re-renders identically, so it's harmless churn).
27+
* - **HTML entity** other than the lowercase canonical `&amp;`/`&lt;`/`&gt;` (e.g. `&copy;`, `&#39;`,
28+
* `&nbsp;`, or the uppercase `&AMP;`) — the serializer escapes the `&`, turning the rendered character
29+
* into literal entity source. The safe-list is deliberately case-*sensitive*: `@tiptap/markdown` only
30+
* round-trips the lowercase forms, so `&AMP;`/`&LT;`/`&GT;` must fall through to read-only rather than
31+
* be treated as safe. A bare `&` with no matching `;`-terminated name is left alone (harmless churn).
3032
*/
3133
const STABLE_LOSS_PATTERNS: ReadonlyArray<RegExp> = [
3234
/^(?=(?:[^\n]*\|){2})[^\n]*<br\s*\/?>/im,
3335
/^#{1,6}\s.*(?: {2,}|\\)$/m,
34-
/&(?!(?:amp|lt|gt);)(?:#x?[0-9a-f]+|[a-z][a-z0-9]*);/i,
36+
/&(?!(?:amp|lt|gt);)(?:#x?[0-9a-fA-F]+|[a-zA-Z][a-zA-Z0-9]*);/,
3537
]
3638

3739
/**
@@ -60,6 +62,40 @@ function linkedImageCount(content: string): number {
6062
return content.match(LINKED_IMAGE_PATTERN)?.length ?? 0
6163
}
6264

65+
/**
66+
* A link/image reference definition line: `[label]: destination "optional title"` (up to 3 leading
67+
* spaces). The `(?!\^)` excludes GFM footnote definitions (`[^id]: …`) — those are preserved verbatim
68+
* by the footnote node and round-trip regardless of whether their reference is present, so they must
69+
* not be treated as droppable orphan definitions.
70+
*/
71+
const REFERENCE_DEFINITION = /^ {0,3}\[(?!\^)([^\]]+)]:[ \t]+\S[^\n]*$/gm
72+
73+
/** CommonMark reference labels match case-insensitively with internal whitespace collapsed. */
74+
function normalizeReferenceLabel(label: string): string {
75+
return label.trim().replace(/\s+/g, ' ').toLowerCase()
76+
}
77+
78+
/**
79+
* True when `content` defines a link/image reference that nothing uses. A *used* reference inlines
80+
* losslessly on serialize (`[x][id]` + `[id]: url` → `[x](url)`), but an *unused* definition is dropped
81+
* entirely — a silent deletion the idempotency probe can't see (the drop happens on the first pass,
82+
* which is then stable). We open such a file read-only rather than lose the definition on first edit.
83+
* Conservative: a label counts as used if it appears bracketed anywhere in the body, so the rare
84+
* inline-text collision errs toward editable, never toward a false read-only.
85+
*/
86+
function hasOrphanReferenceDefinition(content: string): boolean {
87+
const labels = new Set<string>()
88+
for (const match of content.matchAll(REFERENCE_DEFINITION)) {
89+
labels.add(normalizeReferenceLabel(match[1]))
90+
}
91+
if (labels.size === 0) return false
92+
const body = content.replace(REFERENCE_DEFINITION, '').replace(/\s+/g, ' ').toLowerCase()
93+
for (const label of labels) {
94+
if (!body.includes(`[${label}]`)) return true
95+
}
96+
return false
97+
}
98+
6399
/**
64100
* Whether `content` survives the editor's markdown round-trip without data loss or autosave
65101
* churn. The editor opens the content read-only when this is false, so the probe is deliberately
@@ -75,6 +111,7 @@ export function isRoundTripSafe(content: string): boolean {
75111
if (content.length > PROBE_SIZE_LIMIT) return false
76112
const stripped = stripCode(content)
77113
if (STABLE_LOSS_PATTERNS.some((pattern) => pattern.test(stripped))) return false
114+
if (hasOrphanReferenceDefinition(stripped)) return false
78115
try {
79116
const once = serializeMarkdownDocument(content)
80117
if (linkedImageCount(stripped) !== linkedImageCount(stripCode(once))) return false

0 commit comments

Comments
 (0)