fix: don't echo a slash-less path as the login command - #59
Merged
Conversation
Running the CLI from a directory above it made the auth landing page show
an uncopyable command: from '/', the echoed command was
Users/me/dev/onedrive-cli/bin/onedrive login <token>
commandName() prefers a cwd-relative path so the page can suggest
something short like 'bin/onedrive', and guards against paths that escape
the cwd ('..') or come back absolute. But a cwd *above* us produces
neither: Path.relative('/', '/Users/me/.../bin/onedrive') is a perfectly
valid relative path that simply reads as an absolute one with the leading
slash chewed off. Both guards pass and it goes to the page verbatim --
the page renders cmd as-is (' ' + cmd + ' login ' + token), so whatever
lands there has to be runnable on its own.
Cap the relative path at one leading folder, which is all the comment
ever claimed to want, and fall back to the basename beyond that. That
also stops handing an absolute filesystem path to a third-party page in a
query param.
While here: a single-segment relative path was echoed bare, e.g.
'onedrive login <token>' when invoked from inside bin/, which only runs if
that name happens to be on PATH. Emit './onedrive' for that case.
Verified end to end by running `login` without a TTY (which prints the
sign-in URL and the suggested command) from a range of directories, with
matched before/after copies at identical path depths:
cwd before after
/ private/tmp/.../onedrive onedrive
/private/tmp claude-501/.../onedrive onedrive
<repo> bin/onedrive bin/onedrive
<repo>/bin onedrive ./onedrive
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reported from a real sign-in: the auth landing page showed an uncopyable command with the leading slash missing.
Cause
commandName()prefers a cwd-relative path so the page can suggest something short likebin/onedrive, and guards against paths that escape the cwd (..) or come back absolute:A cwd above the script produces neither case.
Path.relative('/', '/Users/me/dev/onedrive-cli/bin/onedrive')is a perfectly valid relative path that simply reads as an absolute one with the leading slash chewed off — both guards pass, and it goes to the page verbatim. The page renderscmdas-is (' ' + cmd + ' login ' + access_token), so whatever lands there has to be runnable on its own.Fix
Cap the relative path at one leading folder — all the function's comment ever claimed to want ("keeps the folder, e.g. 'bin/onedrive'") — and fall back to the basename beyond that.
That also stops handing an absolute filesystem path to a third-party page in a query param. The old behavior leaked
$HOME-revealing paths towww.lunesu.comon every invocation from an ancestor directory; worth noting since the query string may well be logged there.One adjacent case, easy to drop
A single-segment relative path was echoed bare —
onedrive login <token>when invoked from insidebin/— which only runs if that name happens to be onPATH. It now emits./onedrive. Same defect class (the echoed command isn't runnable), but if you'd rather keep this PR to the reported bug, that hunk is two lines and I'm happy to drop it.Testing
Verified end to end rather than by inspection:
loginwithout a TTY prints the sign-in URL and the suggested command, so no network or valid token is needed. Ran matched before/after copies placed at identical path depths, from a range of directories:/private/tmp/…/bin/onedriveonedrive/privatetmp/claude-501/…/bin/onedriveonedrive/private/tmpclaude-501/…/bin/onedriveonedrivebin/onedrivebin/onedrivebin/onedrive./onedriveonedrive(via..guard)onedriveThe intended case (
bin/onedrivefrom the repo root) is unchanged, and the..and absolute-path guards still behave as before.Path.sepis used for both the split and the./prefix, so this stays correct on Windows.Note for the reviewer
npm run lintcannot run onmasterright now (ESLint 10 vs.eslintrc.js) — that is what #55 fixes. I borrowed that PR'seslint.config.jsto lint this branch: no new errors, only the four pre-existing unused catch bindings #55 removes, none of them in this function. Prettier is clean.🤖 Generated with Claude Code