erik/slugkit
1
0
Fork 0
Code Issues 4 Pull requests Projects Releases 21 Packages Wiki Activity Actions

fix: import inline relative Markdown images #294

Merged
erik merged 2 commits from fix/task-6332a4b7-inline-image-import into main 2026-10-04 22:10:23 -05:00
Conversation 2 Commits 2 Files changed 7 +495 -6
erik commented 2026-10-04 21:55:20 -05:00
Owner
Copy link

Summary

  • Resolve local Markdown body images relative to their source file, upload via existing media handling, and rewrite imported content to returned media URLs.
  • Use Markdown-it public APIs for inline/reference images, preserving labels/titles, surrounding Markdown, remote/site URLs, links and code examples.
  • Stable per-post/path keys use the existing server storage-key upsert on re-import. Repeated references share one upload per import; a body reference to the local banner reuses its upload.
  • Pre-read all local body files before mutations; missing/unsupported files fail early. Existing banner/shared-banner and post-by-slug update behavior remains intact.
  • Revalidate decoded paths before any file read/upload: reject absolute paths (including Windows drive/UNC forms), parent-traversal segments and backslashes, matching local banner restrictions.
  • Add latest stable markdown-it 15.0.2 as direct CLI dependency (same library already used by site; built-in types, public APIs). Existing site dependencies and CI workflows remain unchanged. Brief CLI documentation explains import/re-import and path safety.

Verification

  • Original defect reproduced on main with fully mocked fetch: only banner uploaded, two relative body images remained unchanged on create/re-import.
  • Original command regressions failed before implementation; parser and command tests now cover an article with a banner and multiple body images, spaces/encoded filenames, repeat references, shared banner upload, returned CDN URLs, unchanged remote URLs and stable keys on repeat import.
  • Parser coverage includes reference-style images, escaped syntax, inline/fenced/indented code, HTML blocks, nested blocks/tables, images inside links, balanced parentheses, labels/titles, CRLF and failure cases.
  • Independent review requested decoded-path validation. Added seven parser safety cases plus two additional mocked command cases; all nine failed against the initial implementation and now pass. Missing/unsafe body paths cause zero API calls even with a valid local banner.
  • Current verification: ./scripts/pre-pr.sh passed CSS/format/lint/types and 1,114 local tests (533 CLI, 531 site, 50 package tests). CLI build, npm run test:ci and git diff --check passed. npm ci passed before initial pre-PR checks.
  • Existing banner/shared-banner cases and server deterministic-key replacement regression remain green.
  • Evidence: /tmp/slugkit-inline-images-pre-pr.log, /tmp/slugkit-inline-images-build.log, /tmp/slugkit-inline-images-unit-ci.log, /tmp/slugkit-inline-images-safety-red.log and /tmp/slugkit-inline-images-safety-green.log.
  • npm audit reports unchanged pre-existing dependency findings; none of the new cli/node_modules entries is flagged. Unrelated dependency upgrades are outside this task.

All requests/uploads are mocked. No workflow changes or testing infrastructure: NEVER integration tests in CI or release workflows.

Task: #task-6332a4b7

Safety review blocker fixed; awaiting updated CI, independent re-review and explicit human merge approval.

## Summary - Resolve local Markdown body images relative to their source file, upload via existing media handling, and rewrite imported content to returned media URLs. - Use Markdown-it public APIs for inline/reference images, preserving labels/titles, surrounding Markdown, remote/site URLs, links and code examples. - Stable per-post/path keys use the existing server storage-key upsert on re-import. Repeated references share one upload per import; a body reference to the local banner reuses its upload. - Pre-read all local body files before mutations; missing/unsupported files fail early. Existing banner/shared-banner and post-by-slug update behavior remains intact. - Revalidate decoded paths before any file read/upload: reject absolute paths (including Windows drive/UNC forms), parent-traversal segments and backslashes, matching local banner restrictions. - Add latest stable markdown-it 15.0.2 as direct CLI dependency (same library already used by site; built-in types, public APIs). Existing site dependencies and CI workflows remain unchanged. Brief CLI documentation explains import/re-import and path safety. ## Verification - Original defect reproduced on main with fully mocked fetch: only banner uploaded, two relative body images remained unchanged on create/re-import. - Original command regressions failed before implementation; parser and command tests now cover an article with a banner and multiple body images, spaces/encoded filenames, repeat references, shared banner upload, returned CDN URLs, unchanged remote URLs and stable keys on repeat import. - Parser coverage includes reference-style images, escaped syntax, inline/fenced/indented code, HTML blocks, nested blocks/tables, images inside links, balanced parentheses, labels/titles, CRLF and failure cases. - Independent review requested decoded-path validation. Added seven parser safety cases plus two additional mocked command cases; all nine failed against the initial implementation and now pass. Missing/unsafe body paths cause zero API calls even with a valid local banner. - Current verification: ./scripts/pre-pr.sh passed CSS/format/lint/types and 1,114 local tests (533 CLI, 531 site, 50 package tests). CLI build, npm run test:ci and git diff --check passed. npm ci passed before initial pre-PR checks. - Existing banner/shared-banner cases and server deterministic-key replacement regression remain green. - Evidence: /tmp/slugkit-inline-images-pre-pr.log, /tmp/slugkit-inline-images-build.log, /tmp/slugkit-inline-images-unit-ci.log, /tmp/slugkit-inline-images-safety-red.log and /tmp/slugkit-inline-images-safety-green.log. - npm audit reports unchanged pre-existing dependency findings; none of the new cli/node_modules entries is flagged. Unrelated dependency upgrades are outside this task. All requests/uploads are mocked. No workflow changes or testing infrastructure: NEVER integration tests in CI or release workflows. Task: #task-6332a4b7 Safety review blocker fixed; awaiting updated CI, independent re-review and explicit human merge approval.
fix: upload and rewrite local Markdown body images
All checks were successful
CI / build-lint-test (pull_request) Successful in 1m10s
Details
bac20ab2a8 
Resolve images from the source Markdown directory, preserve code and remote references, and reuse deterministic media keys on repeat imports. Share the banner upload path and reuse repeated local files so imported articles retain working image URLs without duplicate media records.

Task: #task-6332a4b7
erik commented 2026-10-04 21:59:31 -05:00
Author
Owner
Copy link

Independent PR review

  • PR: Forgejo #294 (fix/task-6332a4b7-inline-image-import at bac20ab)
  • Task: task-6332a4b7
  • Result: changes requested

Acceptance criteria

  • slug post import uploads local relative images referenced in Markdown body image syntax.
  • Imported Markdown rewrites those image URLs to the returned media URLs.
  • Existing banner import behavior remains intact.
  • Re-importing is predictable and does not create duplicate media or broken post content for the covered source-relative cases.
  • Tests cover an article with a frontmatter banner and multiple inline local images.

Blocking issues

  1. Decoded image paths are not revalidated before filesystem access. cli/src/inline-image-import.ts:71 filters only the raw Markdown destination, then cli/src/inline-image-import.ts:78 decodes it and cli/src/commands.ts:2012 resolves it. Encoded separators or traversal such as %2Ftmp%2Fsecret.png or ..%2Foutside.png can therefore pass local-image classification and be read/uploaded outside the intended source-relative local image scope. Revalidate the decoded path before readMediaUploadFile/upload: reject decoded absolute paths, backslashes/drive-style paths, and .. segments (or otherwise constrain resolved paths to the import source tree) while still allowing ordinary encoded filename characters like spaces.

Warnings

  • None.

Final verdict

Changes requested. The main acceptance criteria are covered, but the decoded-path validation gap is a correctness/security blocker before merge.

### Independent PR review - PR: Forgejo #294 (`fix/task-6332a4b7-inline-image-import` at `bac20ab`) - Task: `task-6332a4b7` - Result: changes requested #### Acceptance criteria - [x] `slug post import` uploads local relative images referenced in Markdown body image syntax. - [x] Imported Markdown rewrites those image URLs to the returned media URLs. - [x] Existing banner import behavior remains intact. - [x] Re-importing is predictable and does not create duplicate media or broken post content for the covered source-relative cases. - [x] Tests cover an article with a frontmatter banner and multiple inline local images. #### Blocking issues 1. Decoded image paths are not revalidated before filesystem access. `cli/src/inline-image-import.ts:71` filters only the raw Markdown destination, then `cli/src/inline-image-import.ts:78` decodes it and `cli/src/commands.ts:2012` resolves it. Encoded separators or traversal such as `%2Ftmp%2Fsecret.png` or `..%2Foutside.png` can therefore pass local-image classification and be read/uploaded outside the intended source-relative local image scope. Revalidate the decoded path before `readMediaUploadFile`/upload: reject decoded absolute paths, backslashes/drive-style paths, and `..` segments (or otherwise constrain resolved paths to the import source tree) while still allowing ordinary encoded filename characters like spaces. #### Warnings - None. #### Final verdict Changes requested. The main acceptance criteria are covered, but the decoded-path validation gap is a correctness/security blocker before merge.
fix: reject unsafe decoded import image paths
All checks were successful
CI / build-lint-test (pull_request) Successful in 54s
Details
6376e54dcf 
Validate local image destinations after URL decoding so encoded absolute paths and parent traversal cannot escape the source directory. Match existing banner restrictions before any file read or upload, with parser and mocked command regressions.

Task: #task-6332a4b7
erik commented 2026-10-04 22:07:55 -05:00
Author
Owner
Copy link

Independent PR re-review

Summary: Re-reviewed Forgejo PR #294 at 6376e54dcf against task task-6332a4b7. The decoded-path safety blocker from the prior review is fixed: readLocalImageToken now validates the decoded local Markdown image path before preparePostImportImages resolves or reads it, rejecting leading /, Windows drive prefixes, backslashes/UNC forms, and .. segments, with parser and mocked command regressions covering the cases.

Acceptance criteria:

  • slug post import uploads local relative images referenced in Markdown body image syntax.
  • Imported Markdown rewrites those image URLs to the returned media URLs.
  • Existing banner import behavior remains intact.
  • Re-importing is predictable and does not create duplicate media or broken post content.
  • Tests cover an article with a frontmatter banner and multiple inline local images.

Blocking issues: none.

Warnings: none.

Final verdict: approved. CI run #500 for 6376e54dcf reports success; local checks were not rerun per review-only instructions.

### Independent PR re-review Summary: Re-reviewed Forgejo PR #294 at `6376e54dcf` against task `task-6332a4b7`. The decoded-path safety blocker from the prior review is fixed: `readLocalImageToken` now validates the decoded local Markdown image path before `preparePostImportImages` resolves or reads it, rejecting leading `/`, Windows drive prefixes, backslashes/UNC forms, and `..` segments, with parser and mocked command regressions covering the cases. Acceptance criteria: - [x] `slug post import` uploads local relative images referenced in Markdown body image syntax. - [x] Imported Markdown rewrites those image URLs to the returned media URLs. - [x] Existing banner import behavior remains intact. - [x] Re-importing is predictable and does not create duplicate media or broken post content. - [x] Tests cover an article with a frontmatter banner and multiple inline local images. Blocking issues: none. Warnings: none. Final verdict: approved. CI run #500 for `6376e54dcf` reports success; local checks were not rerun per review-only instructions.
erik merged commit 9dcd72bc76 into main 2026-10-04 22:10:23 -05:00
erik deleted branch fix/task-6332a4b7-inline-image-import 2026-10-04 22:10:23 -05:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
activitypub

   
admin

   
api

   
architecture

   
articles

   
auth

   
bug

   
cleanup

   
cli

   
comments

   
compatibility

   
config

   
contacts

   
content

   
database

   
deployment

   
design

   
dev-env

   
diagnostics

   
docs

   
documentation

   
email

   
enhancement

   
feature

   
federation

   
feed

   
health

   
homepage

   
implementation

   
integration

   
logging

   
mastodon

   
media

   
navigation

   
npm-packages

   
openapi

   
opengraph

   
packages

   
passkeys

   
preview

   
priority:high

   
priority:low

   
priority:medium

   
production

   
proof

   
proxy

   
public-routes

   
public-ui

   
release

   
safety

   
slug-api

   
slug-auth

   
slug-core

   
slug-federation

   
slug-media

   
slug-template

   
social

   
sources

   
status:active

   
status:canceled

   
status:done

   
status:inprogress

   
status:waiting

   
syndication

   
tailwind

   
template

   
test

   
web

   
webauthn
No labels
activitypub
admin
api
architecture
articles
auth
bug
cleanup
cli
comments
compatibility
config
contacts
content
database
deployment
design
dev-env
diagnostics
docs
documentation
email
enhancement
feature
federation
feed
health
homepage
implementation
integration
logging
mastodon
media
navigation
npm-packages
openapi
opengraph
packages
passkeys
preview
priority:high
priority:low
priority:medium
production
proof
proxy
public-routes
public-ui
release
safety
slug-api
slug-auth
slug-core
slug-federation
slug-media
slug-template
social
sources
status:active
status:canceled
status:done
status:inprogress
status:waiting
syndication
tailwind
template
test
web
webauthn
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
erik/slugkit!294
No description provided.