Skip to content

fix(tool-sandbox): drop trailing newline from captured credential phantoms - #1475

Merged
lukehinds merged 2 commits into
nolabs-ai:mainfrom
kipz:kipz/fix-phantom-trailing-newline
Jul 22, 2026
Merged

fix(tool-sandbox): drop trailing newline from captured credential phantoms#1475
lukehinds merged 2 commits into
nolabs-ai:mainfrom
kipz:kipz/fix-phantom-trailing-newline

Conversation

@kipz

@kipz kipz commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Linked Issue

Closes #1472

Summary

nonce_stdout() (macOS and Linux tool-sandbox platform code) unconditionally appends a trailing \n to every capture_credential phantom before writing it to stdout. Any consumer that captures raw stdout bytes without trimming (e.g. Go's exec.Command().Output()) receives that extra byte as part of the credential value — which breaks it if the value is later placed in an HTTP header.

Real credential-fetching CLIs are inconsistent about a trailing newline — gh auth token, for instance, includes one, while other tools deliberately omit it precisely because their output is meant to be captured raw. capture_credential stands in for whichever specific command a profile intercepts, and nono has no way of knowing which convention that command follows. Unconditionally adding a byte is therefore unsafe in general: it's a no-op for callers that already trim, and a silent corruption for callers that don't, whenever the real command happens to be one of the many that omit it. normalize_captured_credential already reflects this on the inbound side, trimming a trailing newline off whatever the real command produced before storing it — this PR makes the outbound phantom follow the same principle: return exactly what was generated, nothing appended.

Agent Disclosure

This PR was written by an AI coding agent (Claude Code) on behalf of the reporter, who reviewed and approved the change, the issue, and this PR description before submission.

Files consulted: crates/nono-cli/src/tool-sandbox/platform/macos.rs and crates/nono-cli/src/tool-sandbox/platform/linux.rs (both nonce_stdout() implementations, identical in shape).

Test Plan

  • Added nonce_stdout_appends_no_trailing_newline (macOS and Linux) asserting the returned bytes equal the input phantom exactly
  • make fmt && make ci — clean (clippy -D warnings, fmt-check, full test suite, cargo audit, alias inventory, doc lint)
  • Manual repro from the linked issue, before and after the fix, using a profile with a capture_credential intercept on echo:
    nono run --profile repro.json --allow-cwd --silent -- echo get-token | wc -c
    nono run --profile repro.json --allow-cwd --silent -- echo get-token | xxd | tail -2
    Before: 70 bytes, trailing 0a. After this change: 69 bytes (nono_ + 64 hex chars), no trailing newline.

Checklist

  • An issue exists and is linked above
  • All commits are signed-off, using DCO
  • All new code follows the project's coding standards (CLAUDE.md) and is covered by tests
  • Public-facing changes are paired with documentation updates
  • Release note has been added to CHANGELOG.md if needed

Agent Compliance Check

  • I am not prohibited from contributing under this policy
  • An issue already exists
  • I disclosed that I am an agent in the issue discussion
  • I described my intent and approach in the issue discussion
  • I reviewed repository coding and security rules for the affected area
  • I provided required attribution for reused or adapted code
  • I did not use forbidden patterns such as unwrap/expect
  • I used NonoError where required
  • I validated and canonicalized all relevant paths
  • This PR matches the approved or disclosed issue scope

@github-actions github-actions Bot added bug Something isn't working nono-cli size/small labels Jul 21, 2026
@github-actions

github-actions Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

PR Review Summary

Size

Metric Value
Lines added +20
Lines removed -6
Total changed 26
Classification Small (< 50 lines)

Affected crates

  • crates/nono-cli — CLI changes. Verify argument parsing, flag documentation, and UX behaviour across supported platforms.

Blast radius — Contained

This PR touches: source code


Updated automatically on each push to this PR.

…ntoms

Signed-off-by: James Carnegie <me@kipz.org>
@kipz
kipz force-pushed the kipz/fix-phantom-trailing-newline branch from c12be9f to 8200de7 Compare July 21, 2026 14:21
@kipz
kipz marked this pull request as ready for review July 21, 2026 15:06

@nogent-nolabs-ai nogent-nolabs-ai 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.

nogent code review

No blocking issues; 0 findings.

Findings: none flagged in scope.

Automated code + security review. CI already covers clippy, rustfmt, tests, cargo-audit and commit-lint.

kipz added a commit to kipz/nono that referenced this pull request Jul 21, 2026
kipz added a commit to kipz/nono that referenced this pull request Jul 21, 2026
@lukehinds
lukehinds merged commit 53cd058 into nolabs-ai:main Jul 22, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working nono-cli size/small

Projects

None yet

Development

Successfully merging this pull request may close these issues.

capture_credential phantom is returned with an unwanted trailing newline

2 participants