Skip to content

fix(registry): judge a credential helper by its reply when it skips its input - #640

Closed
roodboi wants to merge 1 commit into
nextfrom
claude/credential-helper-epipe
Closed

fix(registry): judge a credential helper by its reply when it skips its input#640
roodboi wants to merge 1 commit into
nextfrom
claude/credential-helper-epipe

Conversation

@roodboi

@roodboi roodboi commented Nov 7, 2026 •
edited by blacksmith-sh Bot
Loading

Copy link
Copy Markdown
Contributor

What

When Hack asks a Docker credential helper for registry credentials, it writes the server address to the helper's stdin. If the helper answered and exited before reading that input, the write failed with a broken pipe and Hack refused the lookup with "Cannot send the server address to the Docker credential helper."

Docker runs helpers through Go's os/exec, which ignores that broken pipe. So whether a pull worked depended on timing. The same race made identity_tokens_use_the_oauth_refresh_grant_and_basic_challenges_use_helpers fail intermittently on loaded CI runners (Candidate core, ubuntu-latest).

A broken pipe on that write is no longer an error: the helper's exit status and reply decide the result, as in Docker. Any other write error still refuses.

Validation

  • New unit test covering the write-error classification.
  • cargo clippy --all-targets -D warnings is clean, and the registry tests pass (32).
  • The previously flaky identity-token test passed 15 of 15 runs.
  • gitleaks is clean.

🤖 Generated with Claude Code

Run Proof
Open in [code]smith · Need help on this PR? Tag @codesmith-bot with what you need.

@roodboi @claude
fix(registry): judge a credential helper by its reply when it skips i…
1866c70 
…ts input

Docker runs credential helpers through Go's os/exec, which ignores a broken
pipe when the helper exits without reading the server address it was given.
Hack refused that case, so a helper that answered and exited quickly could
fail a registry pull depending on timing, and the registry identity-token
test failed intermittently on loaded CI runners. A broken pipe on that write
is no longer an error; the helper's exit status and reply decide the result.
Other write errors still refuse.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@roodboi roodboi mentioned this pull request Nov 7, 2026
Merged
@roodboi

roodboi commented Nov 7, 2026

Copy link
Copy Markdown
Contributor Author

Merged into next through the train batch PR #639 (squash commit e59fcc2). This PR's reviewed head 1866c70f35fc433321292650f366e39f5bf57a5d was included. #639 describes the integration resolutions and the batch security fixes: Go-style image config key reading, non-ASCII key refusal, the build base size bound (unpacked size tracked in HACK-1945), build-base descriptor validation and case-insensitive stage names. Closing as merged by batch; the branch is kept.

🤖 Addressed by Claude Code

@roodboi roodboi closed this Nov 7, 2026
@roodboi roodboi mentioned this pull request Nov 7, 2026
Merged
roodboi added a commit that referenced this pull request Nov 7, 2026
@roodboi @claude
fix(native): merge train batch 7 (#636, #638, #637) (#641)
76460f8 
## What this is

Merge train batch 7 into `next`: three real-project trial fixes. Each
component PR's reviewed head is merged with `git merge --no-ff`, in this
order (#638 is stacked on #636). Integration conflicts are resolved in
the batch's merge commits, described below. Hosted CI on this branch is
the full-suite gate for the combined result.

| PR | Title | Linear | Head included | Review at that head |
| --- | --- | --- | --- | --- |
| #636 | fix(native): compile host aliases like execution and name
unbound aliases | HACK-1939 | `4d25fa5a` | Codex code + security: no
findings; 0 open threads |
| #638 | fix(native): run stock image processes in wrappers like Docker,
naming refusals | HACK-1939 | `08f2d8c3` | Codex code + security: no
findings; 0 open threads |
| #637 | fix(native): reconcile edited graphs that have a dependency
cache | HACK-1937 | `592a9b1d` | Codex code + security: no findings (the
security review confirms the cache-poisoning fix); 0 open threads;
real-VM proof passed |

## Integration changes

- **#636 with batches 1–6:**
- The restart preflight and native `up` pass both #608's
`dependencyOwners` and #636's `internalExtraHostSources`.
- #615 and #636 both added a "nothing was reserved" startup report. They
are now one `finalInspectionFailure`: #615's check plus #636's guards (a
settled failure, not a restore, no mapping attempted), with the combined
message "nothing was created and no cleanup is needed" followed by
#615's diagnostic.
- #636 matched `native_graph_unreserved`, but image projects inspect
with `graph inspect`, which only returns `graph_unreserved`. #636's
check therefore could never fire, so its tests now use
`graph_unreserved`.
- #636's ready summary and confirmed-stop line ride #611's progress
events, so `up` prints one ready line, `Project is ready (SERVICES; run
RUN). …`, with HTTPS URLs and published ports under it. Ctrl-C prints
one stopped line, which states that data was retained when a run mapping
was published.
- **#638 with #615 / #622:** the error type keeps the fixed-message type
and adds #638's process-refusal types. The unified startup report
appends #638's named process refusals after the startup diagnostic.
Doctor keeps the amd64 emulation and settings checks next to #638's
process-wrapper check.
- **#637 with #615 / #638:** failures keep the fixed message, service
and process refusals, and add #637's `planChange`. Its guidance replaces
"inspect owned state before retrying". The cache-poisoning fix holds in
the merged code:
  - `dependency_cache.rs` is #637's file unchanged;
- `plan_reconcile::ordinary_layout` still routes caches with a writable
consumer through reconciliation;
- no batch change mounts a cache volume outside
`instance_scoped_volumes`.
- **Size fix (`dbcf2bcc`):** the three error additions together pushed
`CandidateError` past clippy's large-error bound. `cause_code` is now a
boxed `str`, and process refusals and the plan change share one boxed,
flat-serialized `RefusalDiagnostics`. A test pins the size and the JSON
shape.
- **#640** (credential-helper EPIPE race fix) was folded into batch 6,
so it is not listed here.

## Validation on this branch (`458e0a93`)

- Clean CLI typecheck: pass. Biome on changed files: no errors.
- `RUSTUP_TOOLCHAIN=1.97.1 bun run check:local`: pass. All-features
clippy `-D warnings` (Zig 0.15.2): pass.
- Cargo `--lib`: 1375 pass. `--features native-config-plan`
`provider::graph`: 688 pass. Contracts: `project_contract` 66,
`compose_fields_contract` 10, `interpolation_budget_contract` 1,
`local_contract` 14.
- `bun run test:models` (TLA+ 1.7.4): exit 0, 177 controls verified.
- Focused bun tests, bounded runner, the changed test files: 260 pass, 0
fail.
- `docs:prepare` + `docs:check`, `privacy:check`, gitleaks over
`origin/next..HEAD`: pass.

Not run locally: the full bun suite, the Linux cargo jobs, docker-e2e
and native host qualification. Hosted CI covers the first three.

After this merges, each component PR is closed with a link to the merge
commit.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

<!-- codesmith:footer -->
---
<a
href="https://backend.blacksmith.sh/track/run-proof?expires=1796634294&installation_model_id=17053&pr_number=641&ref=codesmith_pr_footer&repository=hack-dance%2Fhack&return_to=https%3A%2F%2Fgithub.com%2Fhack-dance%2Fhack%2Fpull%2F641&variant=run_proof_on_this_pr&signature=daf0979b3cf0e8fb0d4344fa34460d87b7d75f22ce9fa22f3da3d8c483a575a9"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://pr-comments-assets.blacksmith.sh/codesmith/proof-run-proof-on-this-pr-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://pr-comments-assets.blacksmith.sh/codesmith/proof-run-proof-on-this-pr-light.svg"><img
alt="Run Proof on this PR"
src="https://pr-comments-assets.blacksmith.sh/codesmith/proof-run-proof-on-this-pr-dark-3x.png"
width="168" height="28"></picture></a>
<sup><a
href="https://app.blacksmith.sh/hack-dance/codesmith/hack/pr/641?autoLogin=true&ref=codesmith_pr_footer">Open
in [code]smith</a> · Need help on this PR? Tag
<code>@codesmith-bot</code> with what you need.</sup>

<!-- codesmith:autofix:disabled -->
<!-- /codesmith:footer -->

---------

Co-authored-by: Dimitri Kennedy <344842+roodboi@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@roodboi