Skip to content

Commit e6d9bfa

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 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.
1 parent b09e0c0 commit e6d9bfa

2 files changed

Lines changed: 46 additions & 4 deletions

File tree

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,16 @@ 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)
5868
})
5969

6070
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: 36 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,35 @@ function linkedImageCount(content: string): number {
6062
return content.match(LINKED_IMAGE_PATTERN)?.length ?? 0
6163
}
6264

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

0 commit comments

Comments
 (0)