Skip to content

fix(media): serve and render .md files with inline Markdown preview#6277

Open
webtecnica wants to merge 3 commits into
nesquena:masterfrom
webtecnica:fix/md-media-preview
Open

fix(media): serve and render .md files with inline Markdown preview#6277
webtecnica wants to merge 3 commits into
nesquena:masterfrom
webtecnica:fix/md-media-preview

Conversation

@webtecnica

Copy link
Copy Markdown
Contributor

Problem

MEDIA: links for Markdown (.md) files intermittently download a JSON {"error":"Authentication required"} file on desktop, instead of the actual file.

Root cause: .md was missing from MIME_MAP → fell back to application/octet-stream, and the frontend only showed a bare <a download> link (no inline preview). When the auth cookie wasn't included in the download request, the 401 JSON response was saved as the downloaded file.

Changes

1. MIME mapping (api/config.py)

Add .md, .mkd, .mkdntext/markdown so the server returns the correct Content-Type.

2. Session allow-list (api/routes.py)

Add "text/markdown" to _SESSION_MEDIA_TOKEN_TYPES so .md files pass the session-token media check (alongside the existing root-allowlist check).

3. Frontend Markdown inline preview (static/ui.js + static/style.css)

  • New _MD_EXTS regex constant for .md/.mkd/.mkdn
  • _inlineMediaHtmlForRef renders .md files as a loading placeholder (<div class="md-inline-load">) instead of a bare <a download> link
  • New loadMarkdownInline() fetches the file content via api/media, renders it through the existing renderMd() pipeline, and inserts it inline — same pattern as CSV/HTML/excalidraw previews
  • Added to postProcessRenderedMessages()
  • CSS for loading spinner, wrapped preview container, header bar, and scrollable content area (256 KB cap)

Behavior after fix

Before After
Plain .md download link → auth failure downloads 401 JSON Renders Markdown inline, formatted via the same renderMd() pipeline as chat messages
Content-Type: application/octet-stream Content-Type: text/markdown
No preview, no error feedback Shows loading state, fallback to download on error/overflow

Closes common issue where clicking MEDIA: links for .md files could
download a JSON auth error on desktop. Three-layer fix:

1. MIME mapping (api/config.py): add .md/.mkd/.mkdn → text/markdown
   so the server returns the correct Content-Type instead of
   application/octet-stream

2. Session allow-list (api/routes.py): add text/markdown to
   _SESSION_MEDIA_TOKEN_TYPES so .md files are accepted by the
   session-token path check

3. Frontend preview (static/ui.js + static/style.css): detect .md
   files in _inlineMediaHtmlForRef, fetch content via api/media,
   render through the existing renderMd() pipeline, and display
   inline in a styled container — same pattern as HTML/CSV previews
@greptile-apps

greptile-apps Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a bug where .md file links could download a 401 JSON file instead of the actual content by adding proper MIME mappings, session token allowlist entries, and a full inline Markdown preview renderer to the frontend.

  • api/config.py: Adds .md, .mkd, .mkdntext/markdown to MIME_MAP, fixing the root application/octet-stream fallback.
  • api/routes.py: Adds text/markdown to _SESSION_MEDIA_TOKEN_TYPES so session-token auth works for these files; _INLINE_PREVIEW_TYPES is intentionally left unchanged since the JS preview fetches the text body directly.
  • static/ui.js + static/style.css: Introduces loadMarkdownInline(), which fetches the file content and renders it through the existing renderMd() pipeline, with a 256 KB size cap and CSS matching the HTML/PDF preview pattern.

Confidence Score: 5/5

Safe to merge — the MIME and session-token changes are narrowly scoped, and the JS preview reuses the well-tested renderMd pipeline.

All four changed files make conservative, targeted additions. The MIME mapping and session-token allowlist entries are simple set additions with no branching risk. The frontend renderer delegates to the existing renderMd sanitization pipeline rather than inserting raw file content. The only non-trivial concern is cosmetic: the preview container has no height cap, but that does not affect correctness or security.

static/style.css — the missing max-height on .md-inline-content is the only item worth a second look before merge.

Important Files Changed

Filename Overview
api/config.py Adds .md, .mkd, .mkdntext/markdown MIME entries; straightforward and correct.
api/routes.py Adds text/markdown to _SESSION_MEDIA_TOKEN_TYPES; _INLINE_PREVIEW_TYPES correctly left unchanged since the JS preview uses fetch/text() rather than the browser's inline rendering path.
static/style.css Adds CSS for the markdown inline preview; .md-inline-content has no max-height, so very long files under the 256 KB cap render without a scrollable bound.
static/ui.js Adds _MD_EXTS, loadMarkdownInline(), and hooks into postProcessRenderedMessages; correctly mirrors the CSV/HTML loader pattern, but fallback/error downloadUrl embeds the session_id and i18n keys are missing (both flagged in prior review threads).

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Browser
    participant UI as ui.js
    participant Server as api/routes.py

    Note over Browser,Server: postProcessRenderedMessages() fires after render
    UI->>UI: loadMarkdownInline(container)
    UI->>UI: find .md-inline-load[data-path] elements
    UI->>Server: "fetch(api/media?path=...&session_id=...)"
    Server->>Server: _session_media_token_allows_path()
    Server->>Server: MIME_MAP[".md"] returns text/markdown
    Server-->>UI: 200 OK, Content-Type: text/markdown
    alt "text.length <= 256 KB"
        UI->>UI: renderMd(text)
        UI->>Browser: outerHTML set to md-inline-wrap
    else "text.length > 256 KB"
        UI->>Browser: outerHTML set to md-inline-fallback
    end
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Browser
    participant UI as ui.js
    participant Server as api/routes.py

    Note over Browser,Server: postProcessRenderedMessages() fires after render
    UI->>UI: loadMarkdownInline(container)
    UI->>UI: find .md-inline-load[data-path] elements
    UI->>Server: "fetch(api/media?path=...&session_id=...)"
    Server->>Server: _session_media_token_allows_path()
    Server->>Server: MIME_MAP[".md"] returns text/markdown
    Server-->>UI: 200 OK, Content-Type: text/markdown
    alt "text.length <= 256 KB"
        UI->>UI: renderMd(text)
        UI->>Browser: outerHTML set to md-inline-wrap
    else "text.length > 256 KB"
        UI->>Browser: outerHTML set to md-inline-fallback
    end
Loading

Reviews (3): Last reviewed commit: "fix: keep session_id in download URL for..." | Re-trigger Greptile

Comment thread static/ui.js
Comment thread static/ui.js
Comment on lines +18662 to +18664
const MD_MAX_SIZE = 256 * 1024;

function loadMarkdownInline(container){

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 MD_MAX_SIZE is declared at module scope while HTML_MAX_SIZE in loadHtmlInline (and PDF_MAX_SIZE in loadPdfInline) are declared inside their respective functions. Keeping the constant local to loadMarkdownInline makes the coupling obvious and matches the convention used by every other inline loader.

Suggested change
const MD_MAX_SIZE = 256 * 1024;
function loadMarkdownInline(container){
function loadMarkdownInline(container){
const MD_MAX_SIZE = 256 * 1024; // 256 KB cap for inline Markdown preview

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

…nesquena#6277)

- downloadUrl now built from publicMediaUrl instead of mediaUrl
  (matching HTML/PDF/excalidraw loaders convention)
- MD_MAX_SIZE at module scope kept — consistent with CSV_MAX_SIZE,
  HTML_MAX_SIZE, EXCALIDRAW_MAX_SIZE, PDF_MAX_SIZE
@webtecnica

Copy link
Copy Markdown
Contributor Author

✅ Ajusted: download link usa publicMediaUrl + '&download=1' agora (sem session_id), consistente com HTML/PDF/excalidraw loaders. O MD_MAX_SIZE mantive no escopo do módulo — é o mesmo padrão de CSV_MAX_SIZE, HTML_MAX_SIZE etc.

Comment thread static/ui.js
Comment on lines +2665 to +2666
if(_MD_EXTS.test(ref)){
return `<div class="md-inline-load" data-path="${esc(ref)}"><span class="md-preview-spinner">⏳</span> ${esc(typeof t==='function'?t('md_loading'):'Loading')}...</div>`;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Missing i18n keys produce raw key names in the UI

t('md_loading'), t('md_too_large'), and t('md_error') are called throughout _inlineMediaHtmlForRef and loadMarkdownInline, but none of these keys are registered in i18n.js. The t() function's final fallback (line 26001 of i18n.js: if (val === undefined) return key;) means every user will literally see "md_loading" in the loading spinner, "md_too_large" in the size-cap path, and "md_error" in the catch block — permanently, not just transiently. Compare html_loading, html_too_large, and html_error, which are defined across all supported locales. The hardcoded English strings in the ternary ('File too large for preview', 'Loading', 'Error loading markdown') are dead code because t is always a function in production.

@cutter-sh

cutter-sh Bot commented Jul 18, 2026

Copy link
Copy Markdown

🎬 Cutter preview — PR #6277

/chat
/chat — Markdown files now render inline in chat with a titled preview card and download link.
View Markdown file inline
View Markdown file inline — Missing .md files fall back to a download link with an error note.

@nesquena-hermes nesquena-hermes added the size:M Medium PR (≤10 files, ≤250 LOC) label Jul 19, 2026
@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Summary

This one-line change regresses the authorization path for Markdown artifacts outside the globally allowed media roots. The preview request keeps session_id, but every rendered download link drops it. For a MEDIA:...md artifact that is accepted only through the session-scoped grant, preview succeeds and download returns 403.

Code references

  • static/ui.js:18669-18673 builds both URLs, then derives downloadUrl from publicMediaUrl. This PR changes the final line away from the session-aware URL.
  • api/routes.py:18767-18776 passes the query session_id into _session_media_token_allows_path; api/routes.py:18947-18949 rejects the request when the file is outside the normal roots and that grant is absent.
  • The existing CSV path preserves the same capability on downloads through _csvMediaUrl at static/ui.js:18355-18359.

The URL construction should remain session-aware:

const publicMediaUrl = 'api/media?path=' + encodeURIComponent(path);
const mediaUrl = publicMediaUrl + (mediaSessionId ? '&session_id=' + encodeURIComponent(mediaSessionId) : '');
const downloadUrl = mediaUrl + '&download=1';

Why this matters

The session identifier here is not cosmetic state. It is the capability that lets _handle_media verify the exact assistant/tool-emitted MEDIA: path. Removing it narrows downloads to files under the active workspace, Hermes home, temporary directory, or configured extra roots, even though the adjacent preview fetch can still display the same file.

Suggested verification

Add a focused browser or JavaScript test for a Markdown MEDIA: path outside the normal roots: assert that both the preview fetch and generated attachment URL include the same encoded session_id. Also cover the no-session case to ensure the query is omitted rather than emitted empty. I did not run code under this read-only triage policy; the finding follows the route authorization condition and the established CSV helper contract.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M Medium PR (≤10 files, ≤250 LOC)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants