Skip to content

fix(security): CLI secrets via stdin/file; cap HN comment fan-out (M5/M7) - #18

Merged
willamhou merged 1 commit into
masterfrom
sec/pr-h-cli-secrets-hn-cap
Jun 24, 2026
Merged

fix(security): CLI secrets via stdin/file; cap HN comment fan-out (M5/M7)#18
willamhou merged 1 commit into
masterfrom
sec/pr-h-cli-secrets-hn-cap

Conversation

@willamhou

Copy link
Copy Markdown
Owner

PR-H of the security-hardening plan — the final PR (M5/M7).

M5 — CLI secret intake

Secret-bearing CLI args (vault store token, auth custom client secret, auth import-cookies cookies) can now be supplied via stdin or a --*-file flag instead of a positional/flag value that leaks into shell history and process listings. A shared read_secret(value, file, what) resolves arg → file → stdin. The explicit arg still works but warns (and never logs the value); an interactive TTY with no piped input errors instead of hanging.

M7 — Hacker News fan-out cap

comments.list previously fetched every kid of a story sequentially (one request → potentially thousands of upstream fetches). It now honors a limit param (default 50, hard max 200), fetches take(limit), and reports has_next on truncation. The action declares limit and its pagination max_limit matches the 200 cap.

Tests

CLI parser tests updated for the optional secret args; read_secret resolution verified e2e (stdin / --token-file / positional-warns); cargo test --workspace green; clippy clean.

Codex-reviewed; the flagged MEDIUM (undeclared limit / pagination-max mismatch) and the TTY-hang nicety are fixed.


This completes the security-hardening plan: 5 HIGH, 7 MEDIUM, 3 LOW from the whole-repo review, across 8 PRs.

🤖 Generated with Claude Code

…/M7)

PR-H of the security-hardening plan.

- M5: secret-bearing CLI args (vault token, OAuth client secret, imported cookies)
  can now be read from stdin or a --*-file flag instead of a positional/flag value
  that leaks into shell history and process listings. The explicit arg still works
  but warns; an interactive TTY with no input errors instead of hanging.
- M7: hackernews comments.list caps the comment fan-out (limit param, default 50,
  hard max 200) instead of fetching every kid, so one request can't spawn thousands
  of upstream fetches. The action declares `limit` and its pagination max matches.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@willamhou
willamhou merged commit 7d08f91 into master Jun 24, 2026
2 checks passed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f0db529baf

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

what: &str,
) -> anyhow::Result<String> {
use std::io::{IsTerminal, Read};
if let Some(v) = value {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Treat '-' as stdin instead of a literal secret

When a caller uses the documented stdin form from docs/design/security-hardening.md:252 (-), this branch treats - as the deprecated explicit secret and returns it before reading stdin. In commands like relais vault store github - < token.txt or relais auth custom --client-secret - ..., the stored OAuth/vault secret becomes the single character - and the piped secret is ignored, so the safe stdin path silently produces unusable credentials.

Useful? React with 👍 / 👎.

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 this pull request may close these issues.

1 participant