Skip to content

BACK-467 - Add local file preview with syntax highlighting and line numbers#634

Open
kuwork wants to merge 1 commit into
MrLesk:mainfrom
kuwork:main
Open

BACK-467 - Add local file preview with syntax highlighting and line numbers#634
kuwork wants to merge 1 commit into
MrLesk:mainfrom
kuwork:main

Conversation

@kuwork
Copy link
Copy Markdown
Contributor

@kuwork kuwork commented May 7, 2026

BACK-467 - Add local file preview with syntax highlighting and line numbers

Summary

Adds a local file preview feature to the Web UI. When viewing a task, users can click local file paths in References, Documentation, or Markdown content (Description / Plan / Notes / Final Summary) to view file contents directly in a modal.

Key features:

  • Full file preview and partial line range preview (e.g., src/server/index.ts:35-39)
  • Automatic language detection from file extension with Prism syntax highlighting
  • Line numbers rendered via CSS counters, with correct offset numbering for partial ranges
  • Markdown files rendered directly; code files wrapped in fenced code blocks via MDEditor.Markdown
  • Graceful fallback to normal browser link behavior when a file does not exist
  • API-level directory traversal prevention (../ rejected); only files within the project root are accessible

Path semantics: paths are always resolved relative to the project root (the directory containing backlog/). Use relative paths such as src/server/index.ts or CLI-INSTRUCTIONS.md. Absolute paths and upward traversal are rejected.

Changes since last review

  • Extracted /api/file-content logic into core/filesystem API (FileSystem.readProjectFile)
  • Replaced unsafe startsWith containment check with robust relative + isAbsolute validation
  • Added 5MB file size limit and directory rejection
  • MermaidMarkdown only intercepts relative links when an onFileClick handler is provided
  • Restored original markdown autolink sanitizer regex; hash links no longer forced to target="_blank"

Related Tasks

closes BACK-467

📋 Important: All PRs must have an associated task in the backlog.

  • If no task exists, create one first using: backlog task create "Your task title"
  • Follow the task guidelines when creating tasks
  • Tasks should be atomic, testable, and well-defined with clear acceptance criteria

Task Checklist

  • I have created a corresponding task in backlog/tasks/
  • The task has clear acceptance criteria
  • I have added an implementation plan to the task
  • All acceptance criteria in the task are marked as completed

Testing

  • Manually clicked local paths in References, Documentation, and Markdown content within the Web UI to verify the modal loads file contents correctly
  • Tested line-range links (e.g., file.ts:10-20) to confirm only the specified range loads and line numbers start at the correct offset
  • Tested non-existent file paths to confirm graceful fallback to normal hyperlink behavior instead of errors
  • Tested illegal paths such as ../etc/passwd to verify the API returns 403/404 and blocks directory traversal
  • Verified language detection and syntax highlighting for various extensions (.ts, .md, .css, etc.)

@kuwork kuwork changed the title BACK-465 - Add local file preview with syntax highlighting and line numbers BACK-467 - Add local file preview with syntax highlighting and line numbers May 7, 2026
Copy link
Copy Markdown
Owner

@MrLesk MrLesk left a comment

Choose a reason for hiding this comment

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

Alex's Agent: Thanks for the PR. I like the underlying idea: being able to preview local files from task references/documentation would be useful in the Web UI. I do not think this implementation is ready to merge yet, though.

Main changes I would ask for:

  • Please do not put project file reading/parsing directly in src/server/index.ts. The new /api/file-content handler currently owns path parsing, traversal policy, file IO, line slicing, and response shaping. That should live behind a small, tested core/filesystem API, with the server only adapting HTTP input/output.
  • The current containment check is unsafe. targetPath.startsWith(rootDir) lets sibling directories with the same prefix through. I reproduced /api/file-content?path=../project-secret/secret.txt returning file contents when the project root was /tmp/.../project. Please use a robust containment check, such as normalized relative path semantics and/or realpath where appropriate, and add regression tests for sibling-prefix traversal, absolute paths, .., and valid in-root paths.
  • The file reader needs safer bounds. Before file.text(), please consider file type/size limits and clear behavior for binary or very large files. This endpoint can otherwise pull arbitrary project-root files into the browser response.
  • MermaidMarkdown now intercepts every relative link even when no onFileClick handler is provided. If the file exists, it prevents default navigation and then does nothing. That affects markdown rendered outside task details and also markdown rendered inside the preview modal. Please only install the preview click behavior when a handler exists, or preserve normal link behavior otherwise.
  • The markdown autolink sanitizer regex changed in a way that appears unrelated and risky: it now excludes hyphens, which can break links like <https://example-site.com>. Hash links are also forced into target="_blank". Please keep existing markdown/link behavior unchanged unless it is directly needed for the file-preview feature.
  • Backlog hygiene needs cleanup: the PR title is fine, but the body says closes BACK-465 while the task added here is BACK-467. Current main already has BACK-465 for an unrelated Windows MCP document issue. The added task also has an empty assignee, no final summary/DoD section, and AC #10 mentions a Tailwind upgrade even though this PR does not change package/lock/style files.

CI is green, and the feature direction is good. I would prefer this come back as a smaller implementation where the filesystem access is centralized and regression-tested, then the Web UI consumes that API without changing unrelated markdown behavior.

@kuwork
Copy link
Copy Markdown
Contributor Author

kuwork commented May 8, 2026

Alex's Agent: Thanks for the PR. I like the underlying idea: being able to preview local files from task references/documentation would be useful in the Web UI. I do not think this implementation is ready to merge yet, though.

Main changes I would ask for:

  • Please do not put project file reading/parsing directly in src/server/index.ts. The new /api/file-content handler currently owns path parsing, traversal policy, file IO, line slicing, and response shaping. That should live behind a small, tested core/filesystem API, with the server only adapting HTTP input/output.
  • The current containment check is unsafe. targetPath.startsWith(rootDir) lets sibling directories with the same prefix through. I reproduced /api/file-content?path=../project-secret/secret.txt returning file contents when the project root was /tmp/.../project. Please use a robust containment check, such as normalized relative path semantics and/or realpath where appropriate, and add regression tests for sibling-prefix traversal, absolute paths, .., and valid in-root paths.
  • The file reader needs safer bounds. Before file.text(), please consider file type/size limits and clear behavior for binary or very large files. This endpoint can otherwise pull arbitrary project-root files into the browser response.
  • MermaidMarkdown now intercepts every relative link even when no onFileClick handler is provided. If the file exists, it prevents default navigation and then does nothing. That affects markdown rendered outside task details and also markdown rendered inside the preview modal. Please only install the preview click behavior when a handler exists, or preserve normal link behavior otherwise.
  • The markdown autolink sanitizer regex changed in a way that appears unrelated and risky: it now excludes hyphens, which can break links like <https://example-site.com>. Hash links are also forced into target="_blank". Please keep existing markdown/link behavior unchanged unless it is directly needed for the file-preview feature.
  • Backlog hygiene needs cleanup: the PR title is fine, but the body says closes BACK-465 while the task added here is BACK-467. Current main already has BACK-465 for an unrelated Windows MCP document issue. The added task also has an empty assignee, no final summary/DoD section, and AC Add subtask 4.7: parse unquoted dates #10 mentions a Tailwind upgrade even though this PR does not change package/lock/style files.

CI is green, and the feature direction is good. I would prefer this come back as a smaller implementation where the filesystem access is centralized and regression-tested, then the Web UI consumes that API without changing unrelated markdown behavior.

1. Code structure — src/server/index.ts

Extracted everything into FileSystem.readProjectFile(rawPath) in src/file-system/operations.ts. The server handler now only adapts HTTP input/output and maps exceptions to status codes.

2. Containment check — directory traversal

Replaced targetPath.startsWith(rootDir) with a robust check using normalized relative paths:

const rel = relative(rootDir, targetPath);
const isInside = !rel.startsWith("..") && !isAbsolute(rel);

Added regression tests for sibling-prefix traversal, absolute paths, .., and valid in-root paths.

3. File safety bounds

Added a 5MB size limit, a directory check via stat, and proper handling for missing files, each mapped to the correct HTTP status code.

4. MermaidMarkdown link interception

LinkComponent now returns a plain <a> when onFileClick is absent. Preview click behavior is only installed when a handler is actually provided.

5. Markdown autolink sanitizer

Restored the original regex (\u0000-\u0020) so hyphenated URLs are preserved. Fixed isExternalLink so hash links (#) are no longer forced into target="_blank".

6. Backlog hygiene

Updated directly in the PR page on GitHub (assignee, removed AC #10, added DoD).

@kuwork
Copy link
Copy Markdown
Contributor Author

kuwork commented May 9, 2026

Hi @MrLesk , this PR is ready to merge. Could you please review and approve when you have a moment?By the way,
the BACK-208 PR is ready for review. It includes the paste-as-markdown feature with image support for Word/Excel content. Thanks!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants