fix: harden OAuth login flow and secure token storage permissions

[ragelink]

Summary

While reviewing the codebase before use, I found a few security issues worth addressing:

• CSRF protection: The OAuth authorization URL had no state parameter, leaving the callback open to cross-site request forgery. This PR generates a cryptographically random state value, includes it in the authorization URL, and validates it in the callback before processing any tokens.
• Localhost binding: The callback HTTP server was listening on all interfaces (0.0.0.0) rather than explicitly on localhost. The exposure window is brief, but there's no reason to accept connections from non-loopback addresses.
• Login timeout: The callback server would wait indefinitely if the user closed the browser without completing auth. Added a 5-minute timeout after which the server closes and an error is shown.
• Error output sanitization: The raw error parameter from the OAuth redirect was previously echoed directly to stderr. Replaced with a fixed string to avoid printing arbitrary server-controlled content.
• Config file permissions: ~/.hrvst/config.json (which stores the OAuth access token) was written without explicit permissions, leaving it world-readable on systems with a permissive umask. Now written with mode: 0o600 and the parent directory created with mode: 0o700.

Test plan

• hrvst login opens browser and completes auth successfully
• Verify ~/.hrvst/config.json is created with 600 permissions (ls -la ~/.hrvst/)
• Verify ~/.hrvst/ directory has 700 permissions
• Tamper with the state param in the callback URL — CLI should abort with "Invalid state parameter"
• Let the login prompt sit for 5+ minutes without completing — CLI should exit with timeout message

[kgajera]

@claude Review this PR

[claude]

Claude finished @kgajera's task in 2m 18s —— View job

---

PR Review

• Read changed files and diff
• Analyze security changes
• Provide review feedback

---

Overall, this is a solid security-hardening PR with well-understood motivations. The changes are clearly described and all five fixes address real vulnerabilities. I found a couple of correctness issues worth addressing before merging.

---

src/commands/login.ts

Bug: state is not validated on the error path

src/commands/login.ts lines 61–63

const error = params.get("error");
if (error) {
  console.error(chalk.red("Authentication error."));
} else {
  const returnedState = params.get("state");
  // ...state validation only happens here
}
// server.close() called unconditionally below

When error is present, the code skips state validation entirely and still closes the server. This means anyone who can reach the callback port can abort a login in progress by sending GET /?error=x — no knowledge of the state value required. The state should be checked before branching on error.

Suggested fix: move state validation before the error check so every request from the OAuth redirect is verified.

Fix this →

Minor: success HTML is shown on state mismatch

src/commands/login.ts lines 67–69

if (returnedState !== state) {
  console.error(chalk.red("Invalid state parameter. Login aborted."));
  res.write(LOGIN_HTML); // "You may now close this window" — misleading on error

The invalid-state path writes the success HTML. The user's browser will show "You may now close this window and return to the CLI" even though authentication failed. A short error page would be clearer.

Minor: saveConfig errors are unhandled in the async request handler

src/commands/login.ts line 79

await saveConfig({ accessToken, accountId: scope.split(":")[1] });

The createServer callback is async, but rejections from it aren't caught — if saveConfig throws, the server won't close and the timeout will fire instead. Wrapping the body in a try/catch and calling server.close() in the catch would give a cleaner failure.

---

src/utils/config.ts

Bug: mode: 0o600 does not update permissions on existing files

src/utils/config.ts lines 35–38

await fs.promises.writeFile(
  await configPath(),
  JSON.stringify(Object.assign({}, existingConfig, config)),
  { mode: 0o600 },
);

On Linux/macOS, fs.writeFile passes mode to the underlying open(2) syscall, where it only takes effect when the file is created (O_CREAT). For an existing file the mode argument is ignored and the file retains its original permissions. Users who already have a ~/.hrvst/config.json written without restricted permissions won't get the fix automatically.

To enforce permissions on existing files, fs.chmod (or fs.fchmod after opening) needs to be called explicitly after the write. Fix this →

---

What looks good

• CSRF state: crypto.randomBytes(16).toString("hex") is a sound source of entropy; 128 bits is more than sufficient.
• Localhost binding: .listen(PORT, "localhost") is exactly the right fix.
• Timeout: the server.on("close", () => clearTimeout(timeout)) pairing correctly prevents the timer from firing after a successful login.
• Error sanitization: replacing chalk.red(error) with a fixed string is the right call.
• Directory mode: mkdir with { mode: 0o700 } works correctly because the directory has no group/other bits; the umask can't weaken it.
• Scope regex: /^harvest:\d+$/ is a clean defensive check before splitting.

---

Summary: Two correctness issues worth fixing — the state bypass on the error path (security) and the chmod gap for existing config files (the fix's primary goal). The rest of the changes are well-implemented.