feat(web): add --auth-file so credentials don't pass through argv - #11
Open
TbusOS wants to merge 1 commit into
Open
feat(web): add --auth-file so credentials don't pass through argv#11TbusOS wants to merge 1 commit into
TbusOS wants to merge 1 commit into
Conversation
serve refuses a non-loopback bind without auth, which is right — binding 0.0.0.0 exposes the store to the local network. But --auth USER:PASS was the only way to satisfy that refusal, and on a shared host argv is world-readable: `ps -eo args` prints every process's full command line to every account unless /proc carries hidepid. So the control and the practice contradicted each other, and a user following the error message verbatim published the secret to the whole box. --auth-file reads USER:PASS from the first non-empty line of a path, putting the secret behind filesystem permissions instead. It warns when the file is group- or other-readable, since a world-readable credential file gives back exactly what the route was meant to gain. --auth and --auth-file are mutually exclusive; --auth stays for single-user machines and its help text now names the exposure. The USER:PASS parse is factored into _parse_auth_pair() so both routes reject the same malformed input with the same message. Refs #10. --auth-prompt (option 2 in the issue) is not in this change.
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.
Implements option (1) from #10 —
--auth-file.What changed
cli/engram/commands/web.py:--auth-file PATH— readsUSER:PASSfrom the first non-empty line. Thesecret sits behind filesystem permissions instead of being published through
argv.serveprintsa warning naming the mode and the
chmod 600fix. A world-readable credentialfile would give back exactly what this route was meant to gain, so it should not
fail silently.
--authand--auth-fileis an error ratherthan a silent precedence rule.
_parse_auth_pair(raw, source)— theUSER:PASSsplit is factored out soboth routes reject the same malformed input with the same wording, with the
error naming which route it came from.
--authhelp text now states that it is visible to other accounts viapson a shared host, so the trade-off shows up at the point of use.
The hard refusal at
web/server.py:101is untouched — a non-loopback bind stillrequires auth, with no bypass. This only adds a safe way to satisfy it.
Verification
Exercised end to end against a real store: a 600-mode credential file, a
non-loopback bind, and a browser session over the LAN.
Not in this change
--auth-prompt(option 2 in web serve: --auth is the only credential route, and it leaks the password via the process list #10) — interactive entry, no argv/env/disk at all.separate discussion about whether it earns its place.
permission warning, and the
--auth/--auth-fileconflict. Happy to add themin this PR if preferred over a follow-up.