feat(auth): support non-interactive login via env vars and --password-stdin - #1463
Conversation
…-stdin Resolve GitHub issue #1462: sasjs auth login was interactive-only, making it unusable in CI pipelines and non-interactive agent environments. Implemented the safest combination of options: - SAS_USERNAME / SAS_PASSWORD env vars: skip the corresponding prompts when set, enabling - --password-stdin flag: reads the password from stdin (like ), avoiding shell history leakage - No --password command-line flag (would leak via shell history / ps output) Precedence: Username: SAS_USERNAME env var > interactive prompt (TTY only) Password: --password-stdin > SAS_PASSWORD env var > interactive prompt (TTY only) When --password-stdin is set, stdin is reserved for the password, so the username MUST come from SAS_USERNAME (interactive prompting is not possible). If no TTY is available and no env var / stdin flag supplies the credential, the function throws with a clear message pointing at the env vars or --password-stdin flag. Credentials flow into the existing getTokensWithPasswordGrant -> saveTokens path — no change to token minting, verification, or persistence. The password is never logged or persisted (same guarantee as the interactive flow). Added 11 new tests covering all credential input paths: - env vars (both, username-only, password-only) - --password-stdin (success, empty stdin error, missing SAS_USERNAME error) - no-TTY fallback errors (missing username, missing password) - precedence (--password-stdin over SAS_PASSWORD) - --password-stdin flag parsing and passing in authCommand.spec.ts
Coverage reportTotal coverage
Show files with reduced coverage 🔻Reduced coverage
Report generated by 🧪jest coverage report action from 8ad917e |
![4gl-reviewer[bot]](https://avatars.githubusercontent.com/in/4587573?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hermes Agent Code Review
Verdict: COMMENT — No blocking issues. Clean, well-tested, security-conscious change. Approving with minor suggestions below.
Critical
None.
Warnings
None.
Suggestions
-
login.ts:21— trailing-whitespace trim is correct, but consider documenting the leading-space decision. The comment notes leading/trailing spaces are preserved, which is the right call (passwords can legitimately contain them). One edge case worth a one-line note:echoappends a single\n, butprintf '%s'sends no trailing newline — both work here because the regex only strips when present. No change required; flagging so the design intent is recorded. -
login.ts:16—--password-stdinon a real TTY blocks until Ctrl+D.for await (const chunk of process.stdin)reads until EOF. If a user runs--password-stdinin an interactive shell without piping, the command will appear to hang waiting for EOF. This matchesdocker login --password-stdinbehaviour and the flag is documented for CI/scripts, so it's acceptable — but consider adding a short warning to the flag description (e.g. "Intended for piped/redirected stdin; in a TTY it will block until EOF.") to set expectations. -
authCommand.ts:62— no short alias for--password-stdin.--insecurehas-i; the new flag has none. Not a problem, but if a short alias is desired for ergonomics, consider--password-stdinonly (verbose is fine for a security-sensitive flag, and avoids accidental use). Leaving as-is is reasonable.
Looks Good
- Security: Password is never logged or persisted beyond the short-lived process. Error messages reference the env-var names and flag, not values.
--password-stdinavoids shell history and process argument lists — good practice, mirrorsdocker login. - Env var naming:
SAS_USERNAME/SAS_PASSWORDare consistent with existing usage inaddCredential.ts(createEnvFileForSas9) andrequest.ts. The PR correctly alignsauth loginwith established conventions. - Precedence logic (
login.ts:94-152): Username (env > TTY prompt, blocked when stdin reserved) and Password (stdin > env > TTY prompt) are correct and well-documented. The no-TTY fallback throws clear, actionable errors pointing users at env vars /--password-stdin— exactly what CI needs. - Tests: 9 new test cases covering env-var-both, env-var-each-alone, stdin happy path, empty stdin, stdin-without-username, no-TTY both, no-TTY username-only, and stdin>env precedence. All 29 tests in the two affected spec files pass. Test isolation is handled well (env var reset + TTY restore in
beforeEach/afterAll, stdin restored infinally). - Docs: Command description and examples updated with the new non-interactive flows.
Reviewed by Hermes Agent (GitHub App)
| } | ||
| // Trim trailing newline/carriage-return only; leading/trailing spaces are | ||
| // intentionally preserved in case a password genuinely contains them. | ||
| return data.replace(/[\r\n]+$/g, '') |
There was a problem hiding this comment.
Suggestion: The trailing \r\n trim is correct. Worth a one-line note that printf '%s' (no trailing newline) and echo (trailing \n) both work here, since the regex only strips when present. The intentional preservation of leading/trailing spaces is the right call for passwords.
| */ | ||
| const readPasswordFromStdin = async (): Promise<string> => { | ||
| let data = '' | ||
| for await (const chunk of process.stdin) { |
There was a problem hiding this comment.
Note: for await (const chunk of process.stdin) reads until EOF. On a real TTY with --password-stdin and no pipe, this blocks until Ctrl+D. This matches docker login --password-stdin and the flag is documented for CI/scripts, but consider adding a brief warning to the flag description so interactive users aren't surprised.
| let pass: string | ||
| if (passwordStdin) { | ||
| pass = await readPasswordFromStdin() | ||
| if (!pass) { |
There was a problem hiding this comment.
Good — empty stdin is rejected with a clear, actionable error including the target name and a concrete example command. This is exactly the right UX for CI debugging.
| // When --password-stdin is set, stdin is reserved for the password, so the | ||
| // username MUST come from the env var — no interactive prompt is possible. | ||
| let user: string | ||
| if (envUser) { |
There was a problem hiding this comment.
Precedence is correct here: env var takes priority over the interactive prompt, and the prompt is correctly suppressed when passwordStdin reserves stdin. The no-TTY / no-env fallback throws a helpful error.
1 participant
Resolves #1462
Summary
sasjs auth loginwas interactive-only, making it unusable in CI pipelines and non-interactive agent environments. This PR implements the safest combination of options from the issue:SAS_USERNAME/SAS_PASSWORDenv vars: skip the corresponding prompts when set--password-stdinflag: reads the password from stdin (likedocker login --password-stdin), avoiding shell history leakage--passwordcommand-line flag (would leak via shell history /psoutput)Precedence
When
--password-stdinis set, stdin is consumed for the password, so the username MUST come fromSAS_USERNAME(interactive prompting is not possible).Non-interactive fallback
If no TTY is available and no env var / stdin flag supplies the credential, the function throws with a clear message pointing at the env vars or
--password-stdinflag.Security
Credentials flow into the existing
getTokensWithPasswordGrant->saveTokenspath — no change to token minting, verification, or persistence. The password is never logged or persisted (same guarantee as the interactive flow).Tests
Added 11 new tests covering all credential input paths:
--password-stdin(success, empty stdin error, missingSAS_USERNAMEerror)--password-stdinoverSAS_PASSWORD)--password-stdinflag parsing and passing inauthCommand.spec.tslogin.tshas 100% line/branch/function coverage.