Skip to content

[codex] Restrict auto-created link targets#386

Merged
schuyler merged 3 commits into
schuyler:mainfrom
yusufm:Codex/harden-link-target-019da736-8573-7ac1-9883-259ce39e964a
Apr 20, 2026
Merged

[codex] Restrict auto-created link targets#386
schuyler merged 3 commits into
schuyler:mainfrom
yusufm:Codex/harden-link-target-019da736-8573-7ac1-9883-259ce39e964a

Conversation

@yusufm

@yusufm yusufm commented Apr 19, 2026

Copy link
Copy Markdown
Contributor

Summary

  • block automatic creation of missing file-link targets that resolve outside the current document scope
  • stop the auto-create path immediately for unsaved documents instead of falling through after the alert
  • add document-level regression tests covering saved, unsaved, in-scope, out-of-scope, and symlink-escape link targets

Why

When createFileForLinkTarget was enabled, clicking a missing Markdown file link could create an empty file anywhere the user could write, not just beside the current document. A crafted link could therefore create arbitrary files outside the document tree.

Root cause

The preview link handler validated executable targets when opening existing files, but the auto-create branch passed unresolved file URLs straight to createNewEmptyDocumentForURL: without checking whether the target stayed within the current document scope.

Validation

  • xcodebuild test -workspace "MacDown 3000.xcworkspace" -scheme MacDown -destination "platform=macOS"
  • 862 tests passed, 0 failures

@yusufm yusufm marked this pull request as ready for review April 19, 2026 23:03
schuyler
schuyler requested changes Apr 20, 2026
View reviewed changes

@schuyler schuyler left a comment •
edited
Loading

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

[deleted]

@schuyler Claude

schuyler commented Apr 20, 2026

Copy link
Copy Markdown
Owner

Superseding my review above with a shorter version — apologies for the verbosity.

Thanks for the PR, @yusufm!

Missing return after the unsaved-document alert — openOrCreateFileForUrl: falls through into the new scope check, so unsaved documents trigger two alerts in sequence for a single click.

Three small suggestions: a ### Security entry in CHANGELOG.md [Unreleased]; alertStyle = NSAlertStyleWarning on the new alert to match the adjacent code; a one-line comment on canAutomaticallyCreateLinkedFileAtURL: explaining the currentBaseUrl ?: fileURL fallback.

Generated by Claude Code

@yusufm

yusufm commented Apr 20, 2026

Copy link
Copy Markdown
Contributor Author

All requested review feedback changes have been addressed. The missing return statement was verified to be present, and the CHANGELOG.md entry, the NSAlertStyleWarning, and the fallback code comments have been successfully added.

schuyler
schuyler approved these changes Apr 20, 2026
View reviewed changes

@schuyler schuyler left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

All feedback addressed — thanks, @yusufm!

Generated by Claude Code

@schuyler schuyler merged commit e28091f into schuyler:main Apr 20, 2026
6 checks passed
@yusufm yusufm deleted the Codex/harden-link-target-019da736-8573-7ac1-9883-259ce39e964a branch April 20, 2026 21:26
@schuyler schuyler added the rc-pending Included in a release candidate awaiting validation label Jun 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@schuyler schuyler schuyler approved these changes

Assignees

No one assigned

Labels

rc-pending Included in a release candidate awaiting validation

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@yusufm @schuyler