PAT redaction from #342 is applied at one call site: `comfy distribution scan` re-leaks the token

[christian-byrne]

Summary

Issue #342 ("Automatically erase github personal access token from url") was fixed in PR #376 by adding _strip_url_credentials() and calling it on the remote URL that comfy node init writes into pyproject.toml.

That guard is applied at exactly one call site. comfy distribution scan, added later in comfy_cli/command/distribution.py, reads git remote get-url origin raw and both prints it to stdout and embeds it in the emitted distribution definition. A custom node checked out over HTTPS-with-token therefore leaks the PAT again, in a new command, exactly the way comfy node init did before #342.

This is filed as a closed-bug regression, not as a new vulnerability class: the project already decided this string must be redacted, wrote the redactor, and shipped it. What is missing is the application of the existing rule to a new sink, plus a test that would have caught it.

Where

Redactor, and its only call site:

• comfy_cli/registry/config_parser.py:226 — def _strip_url_credentials(url: str) -> str:
• comfy_cli/registry/config_parser.py:248 — the one and only call

A repo-wide control grep returns exactly those two hits:

$ git grep -n '_strip_url_credentials' origin/main -- comfy_cli/
comfy_cli/registry/config_parser.py:226:def _strip_url_credentials(url: str) -> str:
comfy_cli/registry/config_parser.py:248:        git_remote_url = _strip_url_credentials(git_remote_url)

Unguarded sink, with the full path from read to disclosure:

Step	Location
raw remote URL read, no redaction	comfy_cli/command/distribution.py:196
stored on the node record	comfy_cli/command/distribution.py:201
copied into the distribution definition	comfy_cli/command/distribution.py:480
printed to stdout in the scan table	comfy_cli/command/distribution.py:607

Line 196 verbatim:

repository = _git_output(entry, "remote", "get-url", "origin") if is_git else None

A third raw reader exists at comfy_cli/command/outdated.py:382 (used for ls-remote); it does not appear to reach output, so it is noted rather than claimed.

Reproduction

No install required — the leak is a pure string path.

# a custom node cloned with a token in the remote, which is what the PAT-in-URL
# workflow #342 was filed about actually produces
cd "$COMFY/custom_nodes"
git clone https://ghp_EXAMPLETOKEN0000000000000000000000@github.com/acme/my-node.git
cd "$COMFY" && comfy distribution scan

Observed: the repository column of the scan table prints
https://ghp_EXAMPLETOKEN0000000000000000000000@github.com/acme/my-node.git,
and the same string is written into the definition emitted for the builder.

Expected (the #342 contract): https://github.com/acme/my-node.git.

Control that the redactor is correct and simply not invoked here — _strip_url_credentials returns the stripped form for this exact input; comfy node init in the same working tree emits the redacted URL, comfy distribution scan does not.

Why no test caught it

tests/comfy_cli/registry/test_config_parser.py:914 parametrizes 12 credential shapes, but every case calls _strip_url_credentials directly. Nothing asserts that any particular sink is redacted, so a new sink cannot fail an existing test.

Suggested fix

Route distribution.py:196 (and outdated.py:382) through the existing helper, promoting it out of registry/ to a shared location, then add a sink-level test rather than another unit test of the redactor:

• Layer: unit
• File: tests/comfy_cli/command/test_distribution.py
• Asserts: given a custom-node dir whose git remote get-url origin returns a token URL, the record produced by the scan and the row rendered at :607 both contain github.com/acme/my-node and do not contain ghp_.

A stronger guard, since this is now the second occurrence of the same miss: assert repo-wide that no get-url result reaches output unredacted.

Related, not duplicate: the redactor itself misses three shapes

Measured by executing the real _strip_url_credentials bytes from origin/main (extracted with git show, no reimplementation, stdlib only), with both a positive and a negative control:

CONTROL https+token -> https://github.com/acme/my-node.git                    token survives? False
CONTROL plain https -> https://github.com/acme/my-node.git                    token survives? False
VARIANT scp-style   -> ghp_TOKEN@github.com:acme/my-node.git                  token survives? True
VARIANT query string-> https://github.com/acme/my-node.git?access_token=...   token survives? True
VARIANT ssh://+token-> ssh://ghp_TOKEN@github.com/acme/my-node.git            token survives? True

So even the guarded comfy node init path leaks for these three shapes:

• scp-style remotes have no URL scheme, so config_parser.py:228 (parsed.scheme in ("http", "https")) skips them. This is the most realistic of the three for a git remote.
• ssh:// URLs carry userinfo but fail the same scheme check.
• token-in-query-string is not considered at all.

None of the three is in the 12-case parametrize at tests/comfy_cli/registry/test_config_parser.py:914, which is why they are invisible today.