Merge 43672850ca into b08debceca

Signed-off-by: bigcat88 <bigcat88@icloud.com>

.coderabbit.yaml

@@ -4,12 +4,12 @@ early_access: false
 tone_instructions: "Only comment on issues introduced by this PR's changes. Do not flag pre-existing problems in moved, re-indented, or reformatted code."
 
 reviews:
-  profile: "chill"
-  request_changes_workflow: false
+  profile: "assertive"
+  request_changes_workflow: true
   high_level_summary: false
   poem: false
   review_status: false
-  review_details: false
+  review_details: true
   commit_status: true
   collapse_walkthrough: true
   changed_files_summary: false
@@ -39,6 +39,14 @@ reviews:
     - path: "**"
       instructions: |
         IMPORTANT: Only comment on issues directly introduced by this PR's code changes.
+        Treat AGENTS.md as mandatory repository policy, not optional style guidance.
+        Flag PR changes that violate AGENTS.md even when the code is otherwise functional.
+        In particular, enforce architecture boundaries, dtype/device/memory rules,
+        interface contracts, import style, no unnecessary try/except blocks, no inline
+        imports, no outbound internet paths in core ComfyUI, and narrow scoped fixes.
+        Prefer direct findings over suggestions when a rule is violated. Only ignore
+        AGENTS.md when it clearly conflicts with a newer explicit maintainer instruction
+        in the PR.
         Do NOT flag pre-existing issues in code that was merely moved, re-indented,
         de-indented, or reformatted without logic changes. If code appears in the diff
         only due to whitespace or structural reformatting (e.g., removing a `with:` block),
@@ -123,5 +131,10 @@ chat:
 
 knowledge_base:
   opt_out: false
+  code_guidelines:
+    enabled: true
+    filePatterns:
+      - files: "AGENTS.md"
+        applyTo: "**"
   learnings:
     scope: "auto"

comfy/sd1_clip.py

@@ -543,18 +543,24 @@ class SDTokenizer:
     def _try_get_embedding(self, embedding_name:str):
         '''
         Takes a potential embedding name and tries to retrieve it.
-        Returns a Tuple consisting of the embedding and any leftover string, embedding can be None.
+        Returns a Tuple consisting of the embedding, the cleaned embedding name, and any leftover string, embedding can be None.
         '''
         split_embed = embedding_name.split()
         embedding_name = split_embed[0]
         leftover = ' '.join(split_embed[1:])
+
+        match = re.search(r'[<\[]', embedding_name)
+        if match is not None:
+            leftover = embedding_name[match.start():] + (" " + leftover if leftover else "")
+            embedding_name = embedding_name[:match.start()]
+
         embed = load_embed(embedding_name, self.embedding_directory, self.embedding_size, self.embedding_key)
         if embed is None:
             stripped = embedding_name.strip(',')
             if len(stripped) < len(embedding_name):
                 embed = load_embed(stripped, self.embedding_directory, self.embedding_size, self.embedding_key)
-                return (embed, "{} {}".format(embedding_name[len(stripped):], leftover))
-        return (embed, leftover)
+                return (embed, embedding_name, "{} {}".format(embedding_name[len(stripped):], leftover))
+        return (embed, embedding_name, leftover)
 
     def pad_tokens(self, tokens, amount):
         if self.pad_left:
@@ -585,7 +591,7 @@ class SDTokenizer:
         tokens = []
         for weighted_segment, weight in parsed_weights:
             to_tokenize = unescape_important(weighted_segment)
-            split = re.split(' {0}|\n{0}'.format(self.embedding_identifier), to_tokenize)
+            split = re.split(r'(?<=\s){}'.format(re.escape(self.embedding_identifier)), to_tokenize)
             to_tokenize = [split[0]]
             for i in range(1, len(split)):
                 to_tokenize.append("{}{}".format(self.embedding_identifier, split[i]))
@@ -595,7 +601,7 @@ class SDTokenizer:
                 # if we find an embedding, deal with the embedding
                 if word.startswith(self.embedding_identifier) and self.embedding_directory is not None:
                     embedding_name = word[len(self.embedding_identifier):].strip('\n')
-                    embed, leftover = self._try_get_embedding(embedding_name)
+                    embed, embedding_name, leftover = self._try_get_embedding(embedding_name)
                     if embed is None:
                         logging.warning(f"warning, embedding:{embedding_name} does not exist, ignoring")
                     else:

comfy/security/path_validator.py

@@ -0,0 +1,54 @@
+"""Centralized path-traversal validation for HTTP-exposed file paths.
+
+Replaces ad-hoc checks (`'..' in filename`, `commonpath` comparisons, etc.)
+that previously lived inline across server.py upload/view endpoints.
+Issue: https://github.com/Comfy-Org/ComfyUI/issues/13742
+"""
+from __future__ import annotations
+
+import os
+from pathlib import Path
+from typing import Union
+
+PathLike = Union[str, os.PathLike]
+
+
+def resolve_safe_path(base_dir: PathLike, *user_parts: PathLike) -> Path | None:
+    """Resolve user-supplied path parts against a trusted base directory.
+
+    Joins ``user_parts`` to ``base_dir``, fully resolves the result (following
+    symlinks), and returns it only if it stays inside ``base_dir``. Returns
+    ``None`` for any unsafe input — null bytes, absolute user parts that escape
+    the base, ``..`` segments that walk above the base, symlinks that point
+    outside the base, or any OS-level resolution failure.
+
+    Callers must use the returned ``Path`` for filesystem operations and must
+    not re-join the original user input afterwards, or the validation is moot.
+    """
+    for part in user_parts:
+        if _has_unsafe_chars(part):
+            return None
+
+    try:
+        base = Path(base_dir).resolve(strict=False)
+    except (OSError, ValueError):
+        return None
+
+    try:
+        joined = base.joinpath(*(os.fspath(p) for p in user_parts))
+        candidate = joined.resolve(strict=False)
+    except (OSError, ValueError):
+        return None
+
+    try:
+        candidate.relative_to(base)
+    except ValueError:
+        return None
+    return candidate
+
+
+def _has_unsafe_chars(part: PathLike) -> bool:
+    s = os.fspath(part)
+    # NUL byte: some platforms truncate paths at \x00, which can defeat
+    # subsequent containment checks performed on the truncated string.
+    return "\x00" in s

comfy/text_encoders/llama.py

@@ -937,22 +937,41 @@ class BaseGenerate:
             return torch.argmax(logits, dim=-1, keepdim=True)
 
         # Sampling mode
-        if repetition_penalty != 1.0:
-            for i in range(logits.shape[0]):
-                for token_id in set(token_history):
-                    logits[i, token_id] *= repetition_penalty if logits[i, token_id] < 0 else 1/repetition_penalty
-
-        if presence_penalty is not None and presence_penalty != 0.0:
-            for i in range(logits.shape[0]):
-                for token_id in set(token_history):
-                    logits[i, token_id] -= presence_penalty
+        if len(token_history) > 0 and (repetition_penalty != 1.0 or (presence_penalty is not None and presence_penalty != 0.0)):
+            token_ids = torch.tensor(list(set(token_history)), device=logits.device)
+            token_logits = logits[:, token_ids]
+            if repetition_penalty != 1.0:
+                token_logits = torch.where(token_logits < 0, token_logits * repetition_penalty, token_logits / repetition_penalty)
+            if presence_penalty is not None and presence_penalty != 0.0:
+                token_logits = token_logits - presence_penalty
+            logits[:, token_ids] = token_logits
 
         if temperature != 1.0:
             logits = logits / temperature
 
         if top_k > 0:
-            indices_to_remove = logits < torch.topk(logits, top_k)[0][..., -1, None]
-            logits[indices_to_remove] = torch.finfo(logits.dtype).min
+            top_k = min(top_k, logits.shape[-1])
+            logits, top_indices = torch.topk(logits, top_k)
+
+            if min_p > 0.0:
+                probs_before_filter = torch.nn.functional.softmax(logits, dim=-1)
+                top_probs, _ = probs_before_filter.max(dim=-1, keepdim=True)
+                min_threshold = min_p * top_probs
+                indices_to_remove = probs_before_filter < min_threshold
+                logits[indices_to_remove] = torch.finfo(logits.dtype).min
+
+            if top_p < 1.0:
+                sorted_logits, sorted_indices = torch.sort(logits, descending=True)
+                cumulative_probs = torch.cumsum(torch.nn.functional.softmax(sorted_logits, dim=-1), dim=-1)
+                sorted_indices_to_remove = cumulative_probs > top_p
+                sorted_indices_to_remove[..., 0] = False
+                indices_to_remove = torch.zeros_like(logits, dtype=torch.bool)
+                indices_to_remove.scatter_(1, sorted_indices, sorted_indices_to_remove)
+                logits[indices_to_remove] = torch.finfo(logits.dtype).min
+
+            probs = torch.nn.functional.softmax(logits, dim=-1)
+            next_token = torch.multinomial(probs, num_samples=1, generator=generator)
+            return top_indices.gather(1, next_token)
 
         if min_p > 0.0:
             probs_before_filter = torch.nn.functional.softmax(logits, dim=-1)

comfy_api_nodes/util/request_logger.py

@@ -9,6 +9,7 @@ from typing import Any
 import folder_paths
 
 logger = logging.getLogger(__name__)
+_SENSITIVE_HEADERS = {"authorization", "x-api-key"}
 
 
 def get_log_directory():
@@ -73,6 +74,10 @@ def _format_data_for_logging(data: Any) -> str:
     return str(data)
 
 
+def _redact_headers(headers: dict) -> dict:
+    return {k: ("***" if k.lower() in _SENSITIVE_HEADERS else v) for k, v in headers.items()}
+
+
 def log_request_response(
     operation_id: str,
     request_method: str,
@@ -101,7 +106,7 @@ def log_request_response(
         log_content.append(f"Method: {request_method}")
         log_content.append(f"URL: {request_url}")
         if request_headers:
-            log_content.append(f"Headers:\n{_format_data_for_logging(request_headers)}")
+            log_content.append(f"Headers:\n{_format_data_for_logging(_redact_headers(request_headers))}")
         if request_params:
             log_content.append(f"Params:\n{_format_data_for_logging(request_params)}")
         if request_data is not None:

comfy_extras/nodes_color.py

@@ -16,23 +16,30 @@ class ColorToRGBInt(io.ComfyNode):
             ],
             outputs=[
                 io.Int.Output(display_name="rgb_int"),
-                io.Color.Output(display_name="hex")
+                io.Color.Output(display_name="hex"),
+                io.Float.Output(display_name="alpha"),
             ],
         )
 
     @classmethod
     def execute(cls, color: str) -> io.NodeOutput:
-        # expect format #RRGGBB
-        if len(color) != 7 or color[0] != "#":
-            raise ValueError("Color must be in format #RRGGBB")
+        # expect format #RRGGBB or #RRGGBBAA
+        if len(color) not in (7, 9) or color[0] != "#":
+            raise ValueError("Color must be in format #RRGGBB or #RRGGBBAA")
         try:
             int(color[1:], 16)
         except ValueError:
-            raise ValueError("Color must be in format #RRGGBB") from None
+            raise ValueError("Color must be in format #RRGGBB or #RRGGBBAA") from None
+
+        alpha = 1.0
+        if len(color) == 9:
+            alpha = int(color[7:9], 16) / 255.0
+            color = color[:7]
+
         r, g, b = hex_to_rgb(color)
 
         rgb_int = r * 256 * 256 + g * 256 + b
-        return io.NodeOutput(rgb_int, color)
+        return io.NodeOutput(rgb_int, color, alpha)
 
 
 class ColorExtension(ComfyExtension):

requirements.txt

@@ -1,6 +1,6 @@
 comfyui-frontend-package==1.45.20
 comfyui-workflow-templates==0.11.2
-comfyui-embedded-docs==0.5.6
+comfyui-embedded-docs==0.5.7
 torch
 torchsde
 torchvision

server.py

@@ -38,6 +38,7 @@ from comfy.cli_args import args
 from comfy.deploy_environment import get_deploy_environment
 import comfy.utils
 import comfy.model_management
+from comfy.security.path_validator import resolve_safe_path
 from comfy_api import feature_flags
 import node_helpers
 from comfyui_version import __version__
@@ -406,11 +407,11 @@ class PromptServer():
                     return web.Response(status=400)
 
                 subfolder = post.get("subfolder", "")
-                full_output_folder = os.path.join(upload_dir, os.path.normpath(subfolder))
-                filepath = os.path.abspath(os.path.join(full_output_folder, filename))
-
-                if os.path.commonpath((upload_dir, filepath)) != upload_dir:
+                resolved = resolve_safe_path(upload_dir, subfolder, filename)
+                if resolved is None:
                     return web.Response(status=400)
+                filepath = str(resolved)
+                full_output_folder = str(resolved.parent)
 
                 if not os.path.exists(full_output_folder):
                     os.makedirs(full_output_folder)
@@ -474,10 +475,6 @@ class PromptServer():
                 if not filename:
                     return web.Response(status=400)
 
-                # validation for security: prevent accessing arbitrary path
-                if filename[0] == '/' or '..' in filename:
-                    return web.Response(status=400)
-
                 if output_dir is None:
                     type = original_ref.get("type", "output")
                     output_dir = folder_paths.get_directory_by_type(type)
@@ -485,13 +482,11 @@ class PromptServer():
                 if output_dir is None:
                     return web.Response(status=400)
 
-                if original_ref.get("subfolder", "") != "":
-                    full_output_dir = os.path.join(output_dir, original_ref["subfolder"])
-                    if os.path.commonpath((os.path.abspath(full_output_dir), output_dir)) != output_dir:
-                        return web.Response(status=403)
-                    output_dir = full_output_dir
-
-                file = os.path.join(output_dir, filename)
+                subfolder = original_ref.get("subfolder", "")
+                resolved = resolve_safe_path(output_dir, subfolder, filename)
+                if resolved is None:
+                    return web.Response(status=400)
+                file = str(resolved)
 
                 if os.path.isfile(file):
                     with Image.open(file) as original_pil:
@@ -531,10 +526,6 @@ class PromptServer():
                     if not filename:
                         return web.Response(status=400)
 
-                    # validation for security: prevent accessing arbitrary path
-                    if filename[0] == '/' or '..' in filename:
-                        return web.Response(status=400)
-
                     if output_dir is None:
                         type = request.rel_url.query.get("type", "output")
                         output_dir = folder_paths.get_directory_by_type(type)
@@ -542,14 +533,12 @@ class PromptServer():
                     if output_dir is None:
                         return web.Response(status=400)
 
-                    if "subfolder" in request.rel_url.query:
-                        full_output_dir = os.path.join(output_dir, request.rel_url.query["subfolder"])
-                        if os.path.commonpath((os.path.abspath(full_output_dir), output_dir)) != output_dir:
-                            return web.Response(status=403)
-                        output_dir = full_output_dir
-
+                    subfolder = request.rel_url.query.get("subfolder", "")
                     filename = os.path.basename(filename)
-                    file = os.path.join(output_dir, filename)
+                    resolved = resolve_safe_path(output_dir, subfolder, filename)
+                    if resolved is None:
+                        return web.Response(status=400)
+                    file = str(resolved)
 
                 if os.path.isfile(file):
                     if 'preview' in request.rel_url.query:

tests-unit/comfy_test/security/path_validator_test.py

@@ -0,0 +1,109 @@
+import os
+import sys
+import tempfile
+from pathlib import Path
+
+import pytest
+
+from comfy.security.path_validator import resolve_safe_path
+
+
+@pytest.fixture
+def base():
+    with tempfile.TemporaryDirectory() as tmp:
+        yield Path(tmp).resolve()
+
+
+def test_simple_filename_inside_base(base):
+    result = resolve_safe_path(base, "foo.png")
+    assert result == base / "foo.png"
+
+
+def test_subfolder_plus_filename(base):
+    result = resolve_safe_path(base, "subdir", "foo.png")
+    assert result == base / "subdir" / "foo.png"
+
+
+def test_empty_subfolder(base):
+    result = resolve_safe_path(base, "", "foo.png")
+    assert result == base / "foo.png"
+
+
+def test_dotdot_segment_rejected(base):
+    assert resolve_safe_path(base, "..", "etc") is None
+    assert resolve_safe_path(base, "../etc") is None
+    assert resolve_safe_path(base, "sub/../..", "etc") is None
+
+
+def test_absolute_user_path_rejected(base):
+    if sys.platform == "win32":
+        assert resolve_safe_path(base, "C:\\Windows\\System32") is None
+    else:
+        assert resolve_safe_path(base, "/etc/passwd") is None
+
+
+def test_null_byte_rejected(base):
+    assert resolve_safe_path(base, "foo\x00.png") is None
+    assert resolve_safe_path(base, "sub\x00", "foo.png") is None
+
+
+def test_dotdot_inside_base_is_allowed(base):
+    (base / "sub").mkdir()
+    result = resolve_safe_path(base, "sub", "..", "foo.png")
+    assert result == base / "foo.png"
+
+
+@pytest.mark.skipif(sys.platform == "win32", reason="symlink permissions on Windows CI")
+def test_symlink_escape_rejected(base):
+    outside = Path(tempfile.mkdtemp())
+    try:
+        link = base / "escape"
+        link.symlink_to(outside)
+        assert resolve_safe_path(base, "escape", "secret") is None
+    finally:
+        # cleanup outside dir
+        try:
+            outside.rmdir()
+        except OSError:
+            pass
+
+
+@pytest.mark.skipif(sys.platform == "win32", reason="symlink permissions on Windows CI")
+def test_symlink_inside_base_is_allowed(base):
+    target = base / "real"
+    target.mkdir()
+    link = base / "alias"
+    link.symlink_to(target)
+    result = resolve_safe_path(base, "alias", "foo.png")
+    assert result == (target / "foo.png").resolve()
+
+
+def test_base_canonicalizes_via_resolve(base):
+    # Pass an unresolved base (with trailing slash) and verify containment still works.
+    unresolved = str(base) + os.sep
+    assert resolve_safe_path(unresolved, "foo.png") == base / "foo.png"
+
+
+def test_dot_segments_are_normalized(base):
+    assert resolve_safe_path(base, ".", "foo.png") == base / "foo.png"
+    assert resolve_safe_path(base, "./sub/./foo.png") == base / "sub" / "foo.png"
+
+
+def test_path_objects_accepted(base):
+    assert resolve_safe_path(base, Path("foo.png")) == base / "foo.png"
+
+
+@pytest.mark.skipif(sys.platform != "win32", reason="Windows-specific path separators")
+def test_windows_backslash_traversal_rejected(base):
+    assert resolve_safe_path(base, "..\\etc") is None
+
+
+@pytest.mark.skipif(sys.platform != "win32", reason="Windows-specific UNC paths")
+def test_windows_unc_path_rejected(base):
+    assert resolve_safe_path(base, "\\\\server\\share\\file") is None
+
+
+def test_returns_path_instance(base):
+    result = resolve_safe_path(base, "foo.png")
+    assert isinstance(result, Path)
+    assert result.is_absolute()