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

Shell names are not escaped! #24

Closed
jfly opened this issue Dec 3, 2021 · 0 comments · Fixed by #25
Closed

Shell names are not escaped! #24

jfly opened this issue Dec 3, 2021 · 0 comments · Fixed by #25
Assignees

Comments

@jfly
Copy link
Owner

jfly commented Dec 3, 2021

Check this out:

$ shtuff as /foo
Traceback (most recent call last):
  File "/home/jeremy/.local/bin/shtuff", line 8, in <module>
    sys.exit(main())
  File "/home/jeremy/.local/lib/python3.9/site-packages/shtuff.py", line 83, in main
    func(**args)
  File "/home/jeremy/.local/lib/python3.9/site-packages/shtuff.py", line 93, in shtuff_as
    with open(pid_file, 'w') as f:
PermissionError: [Errno 13] Permission denied: '/foo.pid'

Note how it's trying to create a foo.pid directory at the root of my filesystem. That shouldn't happen!

The problem is that we don't do any escaping of the filename when calling get_pid_file:

$ python
>>> import shtuff
>>> shtuff.get_pid_file(name="/foo")
'/foo.pid'

vs:

>>> shtuff.get_pid_file(name="foo")
'/home/jeremy/.local/share/shtuff/foo.pid'

I think the fix is to have get_pid_file do some escaping of the name. The simplest option may be base64 encode the whole thing? Looking through https://stackoverflow.com/questions/295135/turn-a-string-into-a-valid-filename, https://stackoverflow.com/a/295150 mentions this, and warns to be careful to avoid the / character.

@jfly jfly self-assigned this Dec 3, 2021
jfly added a commit that referenced this issue Dec 3, 2021
jfly added a commit that referenced this issue Dec 3, 2021
@jfly jfly closed this as completed in #25 Dec 3, 2021
jfly added a commit that referenced this issue Dec 3, 2021
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 a pull request may close this issue.

1 participant