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

オープン
  • 何か問題がありますか？
#748 コメント 0 件 リアクション 0 件 担当者 1 名 GitHub で見る

メンテナーはふだん 1 日以内に返信

@sundar-svg がすでに取り組んでいます。

2026年8月21日 から。

評価

この issue はまだ評価されていません。

説明

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.

主要言語
Python
スター
1k
フォーク
176
平均マージ
1日 11時間
マージ済み PR（30日）
87

環境構築

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

Comfy-Org/comfy-cli のほかの issue

Comfy-Org/comfy-cli の issue をすべて見る

似ている issue

Python の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。