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

Aperta
  • Qualcosa non va?
#748 0 commenti 0 reazioni 1 assegnatario Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

@sundar-svg ci sta già lavorando.

Dal 21/8/2026.

Valutazione

Questa issue non è ancora stata valutata.

Descrizione

audit:security bug effort:small

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://[email protected]/acme/my-node.git
cd "$COMFY" && comfy distribution scan

Observed: the repository column of the scan table prints
https://[email protected]/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   -> [email protected]: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://[email protected]/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.

Lingua principale
Python
Stelle
1k
Fork
176
Merge medio
1g 11h
PR unite (30g)
87

Preparare l'ambiente

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di Comfy-Org/comfy-cli

Tutte le issue di Comfy-Org/comfy-cli

Issue simili

Altre issue su Python

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.