Skip to content
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

WIP: make SandboxedPath internals always Abs #54

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

srghma
Copy link
Member

@srghma srghma commented Oct 11, 2024

Description of the change

this pr is on top of #52

what is here:

  1. only appeand, no go up SafeAppend <///> for SandboxedPath
  2. make SandboxedPath not have Rel in type - always Abs to fix all sandbox methods should return Abs Paths? #53
  3. add Pathy.Name.basename

Where I use it? here https://github.com/srghma/purescript-pathy-node , can move to purescript-contrib


Checklist:

  • Added the change to the changelog's "Unreleased" section with a link to this PR and your username
  • Linked any existing issues or proposals that this pull request should close
  • Updated or added relevant documentation in the README and/or documentation directory
  • Added a test for the contribution (if applicable)

@srghma srghma changed the title WIP: Sandboxed data sandboxedpath dirorfile = sandboxedpath path abs dir path abs dirorfile WIP: make SandboxedPath internals always Abs Oct 11, 2024
@srghma srghma force-pushed the sandboxed--data-sandboxedpath-dirorfile-=-sandboxedpath-path-abs-dir-path-abs-dirorfile branch from 2b8079a to 8352f9e Compare October 16, 2024 08:17
@garyb
Copy link
Member

garyb commented Oct 16, 2024

Sandboxing is only meant to guarantee that a user provided path is not escaping some absolute context, it never claimed to make the path absolute as far as I know. But I can see where you're coming from, it does make sense that unsandboxing/printing a sandboxed path would produce an absolute path, unfortunately I don't think that's entirely harmless just now - we'd need to figure out and have the windows absolute paths working correctly too.

Also, since I don't think I've said it elsewhere, thanks for all the work you're putting into things all over the place!

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

Successfully merging this pull request may close these issues.

all sandbox methods should return Abs Paths?
2 participants