Guides image + draft gating, audit retention, ADR-015 gate, backup credential redaction

plugins/guides/frontend/utils/images.js

@@ -0,0 +1,77 @@
+/**
+ * Resolving guide images.
+ *
+ * A guide image lives behind an authenticated route, and an <img src> is a
+ * plain browser request that carries no Authorization header - this app keeps
+ * its JWT in one. Pointing src at the image route therefore yields 401 and a
+ * broken-image icon for every reader, which is how guides first shipped.
+ *
+ * So the bytes are fetched through the api client, which attaches the token and
+ * resolves against its own baseURL - the latter also fixing a stored
+ * '/api/guides/images/N' string under a subpath mount, where the API is not at
+ * /api at all.
+ *
+ * Used by the renderer AND the editor: an author needs to see the screenshot
+ * they just pasted as much as a reader does.
+ */
+import { reactive, watch, onBeforeUnmount } from 'vue'
+import api from '@/api'
+
+/** The image id a block refers to, whichever way it was written.
+ *
+ * Blocks store imageid (written since this was fixed) or a url string (written
+ * before it). The url is never used as a src, only read for its id.
+ */
+export function imageIdOf (block) {
+  if (!block) return ''
+  if (block.imageid) return String(block.imageid)
+  const match = /\/guides\/images\/(\d+)/.exec(block.url || '')
+  return match ? match[1] : ''
+}
+
+/**
+ * Keeps object URLs for the image blocks in `blocksRef`, fetching each id once
+ * however many blocks use it, and revoking them as blocks go away.
+ *
+ * Returns `state(block)` -> { src, failed }.
+ */
+export function useGuideImages (blocksRef) {
+  const images = reactive({})
+
+  async function load (id) {
+    if (images[id]) return
+    images[id] = { src: '', failed: false }
+    try {
+      const response = await api.get(`/guides/images/${id}`, { responseType: 'blob' })
+      images[id].src = URL.createObjectURL(response.data)
+    } catch (err) {
+      images[id].failed = true
+    }
+  }
+
+  function release (id) {
+    if (images[id] && images[id].src) URL.revokeObjectURL(images[id].src)
+    delete images[id]
+  }
+
+  function state (block) {
+    const id = imageIdOf(block)
+    if (!id) return { src: '', failed: true }
+    return images[id] || { src: '', failed: false }
+  }
+
+  // Editing replaces the block list on every keystroke, so resolve only ids
+  // that are new and keep the ones already held. Without that, every keystroke
+  // would refetch every image in the guide.
+  watch(blocksRef, blocks => {
+    const wanted = new Set(
+      (blocks || []).filter(b => b && b.type === 'image')
+        .map(imageIdOf).filter(Boolean))
+    wanted.forEach(load)
+    Object.keys(images).forEach(id => { if (!wanted.has(id)) release(id) })
+  }, { immediate: true, deep: true })
+
+  onBeforeUnmount(() => { Object.keys(images).forEach(release) })
+
+  return { state }
+}

plugins/guides/frontend/utils/inline.js

@@ -27,12 +27,29 @@ function escapeHtml (text) {
     .replace(/'/g, ''')
 }
 
+// The text was HTML-escaped before any pattern ran, so a captured URL arrives
+// carrying entities: a real query string like ?a=1&b=2 reaches here as
+// ?a=1&b=2. Escaping that again for the attribute produced & and a
+// link that opened the wrong record - the ServiceNow and SharePoint links that
+// carry several parameters were all broken this way. Decode first, then let the
+// single escape on output put back exactly one level.
+function decodeEntities (text) {
+  return String(text)
+    .replace(/&lt;/g, '<')
+    .replace(/&gt;/g, '>')
+    .replace(/&quot;/g, '"')
+    .replace(/&#39;/g, "'")
+    .replace(/&amp;/g, '&')   // last, so &amp;lt; does not become <
+}
+
 // Only http(s) and mailto. A javascript: or data: URL in a link is the one way
 // an author could otherwise smuggle script past the escaping above.
 function safeHref (url) {
   const trimmed = String(url || '').trim()
   if (/^(https?:|mailto:)/i.test(trimmed)) return trimmed
-  if (trimmed.startsWith('/')) return trimmed   // in-app link
+  // In-app link. NOT '//host', which looks like a path and is read by the
+  // browser as protocol-relative - an off-site link wearing a local costume.
+  if (trimmed.startsWith('/') && !trimmed.startsWith('//')) return trimmed
   return null
 }
 
@@ -50,13 +67,18 @@ export function renderInline (text) {
   // [label](url). The URL pattern allows ONE level of balanced parentheses so
   // real links survive - Wikipedia-style /wiki/Foo_(bar) is the common case,
   // and a naive [^)]+ silently truncates it into a broken link.
+  // Finished links are stashed like code spans, so the emphasis passes below
+  // cannot reach inside an href. Without that, a URL containing *characters*
+  // has <em> injected into the middle of the attribute and the link breaks.
+  const links = []
   html = html.replace(/\[([^\]]+)\]\(((?:[^()\s]|\([^()\s]*\))+)\)/g, (match, label, url) => {
-    const href = safeHref(url)
+    const href = safeHref(decodeEntities(url))
     if (!href) return label
     const external = /^https?:/i.test(href)
-    return '<a href="' + escapeHtml(href) + '"' +
+    links.push('<a href="' + escapeHtml(href) + '"' +
       (external ? ' target="_blank" rel="noopener noreferrer"' : '') +
-      '>' + label + '</a>'
+      '>' + label + '</a>')
+    return '\u0000LINK' + (links.length - 1) + '\u0000'
   })
 
   // **bold** then *italic*, longest marker first so **x** is not read as
@@ -66,6 +88,8 @@ export function renderInline (text) {
 
   html = html.replace(/\n/g, '<br>')
 
+  html = html.replace(/\u0000LINK(\d+)\u0000/g, (match, index) => links[Number(index)])
+
   html = html.replace(/\u0000CODE(\d+)\u0000/g, (match, index) =>
     '<code>' + codeSpans[Number(index)] + '</code>')
 

plugins/guides/frontend/utils/inline.spec.js

@@ -0,0 +1,50 @@
+/**
+ * Inline rendering: the security model and the link bugs it hid.
+ *
+ * Escape-then-format is the whole defence, and it has one awkward consequence
+ * that produced real broken links: by the time a URL is captured it is already
+ * escaped, so it has to be decoded before it can be escaped again exactly once.
+ */
+import { describe, it, expect } from 'vitest'
+import { renderInline } from './inline.js'
+
+describe('renderInline', () => {
+  it('escapes markup an author pasted', () => {
+    expect(renderInline('<img src=x onerror=alert(1)>'))
+      .toBe('&lt;img src=x onerror=alert(1)&gt;')
+  })
+
+  it('keeps a multi-parameter URL intact', () => {
+    // Double-escaped, this became &amp;amp; and the link opened the wrong
+    // record. Every ServiceNow and SharePoint link carries several parameters.
+    const html = renderInline('[ticket](https://x.example/nav?a=1&b=2&c=3)')
+    expect(html).toContain('href="https://x.example/nav?a=1&amp;b=2&amp;c=3"')
+    expect(html).not.toContain('&amp;amp;')
+  })
+
+  it('does not let emphasis reach inside an href', () => {
+    const html = renderInline('[t](https://x.example/a*b*c)')
+    expect(html).toContain('href="https://x.example/a*b*c"')
+    expect(html).not.toContain('<em>')
+  })
+
+  it('refuses a protocol-relative URL dressed as an in-app path', () => {
+    // '//evil.example' starts with '/' but the browser reads it as off-site.
+    expect(renderInline('[evil](//evil.example/x)')).toBe('evil')
+  })
+
+  it('refuses javascript: and keeps the label', () => {
+    expect(renderInline('[bad](javascript:alert(1))')).toBe('bad')
+  })
+
+  it('allows an in-app link', () => {
+    expect(renderInline('[map](/map)')).toBe('<a href="/map">map</a>')
+  })
+
+  it('still renders bold, italic and code', () => {
+    expect(renderInline('**b**')).toBe('<strong>b</strong>')
+    expect(renderInline('*i*')).toBe('<em>i</em>')
+    expect(renderInline('`C:\\**temp**`'))
+      .toBe('<code>C:\\**temp**</code>')
+  })
+})