Skip to content

Don’t allow source files outside of the source root #71

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Fil opened this issue Oct 26, 2023 · 2 comments · Fixed by #72
Closed

Don’t allow source files outside of the source root #71

Fil opened this issue Oct 26, 2023 · 2 comments · Fixed by #72
Assignees
Labels
bug Something isn’t working

Comments

@Fil
Copy link
Contributor

Fil commented Oct 26, 2023

For example, if a markdown page link.md has: <link href=../../../Downloads/dark.css> the yarn build command should ignore it.

[EDITED] currently we see the file being written:
copy ../../Downloads/dark.css → ../Downloads/dark.css
this ends up, in my case, in /Users/fil/Source/Downloads/dark.css, since I am working from /Users/fil/Source/ci/.

and the yarn dev command returns a page containing <link href="/_file/../../../Downloads/dark.css"> which it doesn't know how to serve because it results in the path /Downloads/dark.css.

If we want to support linking to files outside of docs/, then maybe we need to be more vigilant about the path rewrite; but it would seem simpler and safer to not allow that?

(may be related to #30 #40 #54)

@Fil Fil added the bug Something isn’t working label Oct 26, 2023
@mbostock mbostock changed the title ensure we don't write outside of build/ Don’t allow source files outside of the source root Oct 26, 2023
@cinxmo
Copy link
Contributor

cinxmo commented Oct 26, 2023

I personally don't think we should be able to link any local files outside of our project root directory (currently docs but we can overwrite it)

@mbostock
Copy link
Member

We should error/ignore relative paths that go outside of the source root. You should be required to move the source file into the source root for it to work (and therefore it should be safe to preserve the relative path as-is when copying to the output root).

Fil added a commit that referenced this issue Oct 26, 2023
@Fil Fil closed this as completed in #72 Nov 2, 2023
Fil added a commit that referenced this issue Nov 2, 2023
* A file is local if it exists in the root folder or a subfolder. Work with normalized paths.

closes #71

* apply suggestions from review

* don't path-test refs that start with https://
@cinxmo cinxmo mentioned this issue Nov 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn’t working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants