[BOUNTY #8889] Remove tokens from history items#13095
[BOUNTY #8889] Remove tokens from history items#13095zhaog100 wants to merge 4 commits intoComfy-Org:masterfrom
Conversation
Closes Comfy-Org#8899 - schemas/prompt.json: Draft-07 JSON Schema documenting the prompt format - Node objects with class_type (required), inputs (required), _meta (optional) - Node links as [source_id, output_index] arrays - Self-validating examples included in schema - docs/api/prompt-schema.md: Documentation with validation rules, examples, and common error types Based on analysis of execution.py:validate_prompt() and server.py
Sanitize history items returned by get_history() to strip auth_token_comfy_org and api_key_comfy_org from prompt inputs. This prevents tokens from being exposed if history is ever persisted to disk or accessed over open networks. Closes Comfy-Org#8889
zhaog100
requested review from
Kosinkadink,
comfyanonymous and
guill
as code owners
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds API documentation and a JSON Schema for the ComfyUI 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/api/prompt-schema.md`:
- Around line 39-43: Update the "Direct Value" section (the "Direct Value"
heading and its JSON example) to clarify that while most JSON-serializable
scalars and objects are accepted as direct values, raw arrays are interpreted as
links at runtime and therefore must be wrapped using the {"__value__": [...] }
form when sending a literal array; apply this clarification to both occurrences
of the example (the Direct Value block and the repeated block around lines
45-49) and include a short wrapped-array example for clarity.
In `@execution.py`:
- Around line 1256-1268: _sanitize_history_item currently only handles when the
history item's "prompt" is a dict, but stored history uses a queued prompt as a
tuple/list so sanitization is skipped; update _sanitize_history_item to handle
prompt values that are sequences (tuple/list) by iterating over elements (e.g.,
each node dict) and removing sensitive keys from node_data.get("inputs", {}) the
same way it does for dict prompts, and ensure get_history calls
_sanitize_history_item for each returned item; update references to
"auth_token_comfy_org" and "api_key_comfy_org" removal logic inside
_sanitize_history_item to operate on inputs regardless of whether prompt is a
dict or sequence so sanitization is no-op-proof.
In `@schemas/prompt.json`:
- Around line 38-42: The schema currently lets any value for "inputs" (and
likewise "outputs") via additionalProperties: true, so arrays like ["4"] slip
through; update the "inputs" and "outputs" property schemas to replace
additionalProperties: true with an object schema that uses additionalProperties:
{ anyOf: [ { }, { $ref: "#/definitions/node_link" } ] } (or equivalent) so each
property value is either any valid JSON value or a reference to the existing
"node_link" definition (which enforces exactly two items); ensure the change is
applied to both "inputs" and "outputs" entries and that the "node_link"
definition name is used verbatim.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5a1d99dd-11c4-4df7-9b04-f21103f72331
📒 Files selected for processing (3)
docs/api/prompt-schema.mdexecution.pyschemas/prompt.json
| #### Direct Value | ||
| Any JSON-serializable value: | ||
| ```json | ||
| {"seed": 123456, "steps": 20, "cfg": 7.0, "width": 512} | ||
| ``` |
There was a problem hiding this comment.
Direct-value docs are too broad for array inputs.
The phrase “Any JSON-serializable value” implies raw arrays are valid direct values, but arrays are interpreted as links in runtime validation. This can cause client-side confusion.
Suggested doc tweak
-Any JSON-serializable value:
+Any JSON-serializable value **except raw arrays**:
```json
{"seed": 123456, "steps": 20, "cfg": 7.0, "width": 512}+If you need to send a literal array value, wrap it as:
+json +{"some_input": {"__value__": [1, 2, 3]}} +
</details>
Also applies to: 45-49
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @docs/api/prompt-schema.md around lines 39 - 43, Update the "Direct Value"
section (the "Direct Value" heading and its JSON example) to clarify that while
most JSON-serializable scalars and objects are accepted as direct values, raw
arrays are interpreted as links at runtime and therefore must be wrapped using
the {"value": [...] } form when sending a literal array; apply this
clarification to both occurrences of the example (the Direct Value block and the
repeated block around lines 45-49) and include a short wrapped-array example for
clarity.
</details>
<!-- fingerprinting:phantom:poseidon:hawk -->
<!-- This is an auto-generated comment by CodeRabbit -->
| "inputs": { | ||
| "type": "object", | ||
| "description": "Node inputs. Keys are parameter names. Values are either direct values or node links [source_id, output_index].", | ||
| "additionalProperties": true | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verifies whether malformed link arrays are currently accepted by schemas/prompt.json
# Expected current behavior (before fix): 0 validation errors (incorrectly accepted)
python - <<'PY'
import json
from jsonschema import Draft7Validator
with open("schemas/prompt.json", "r", encoding="utf-8") as f:
schema = json.load(f)
bad_prompt = {
"1": {
"class_type": "KSampler",
"inputs": {
"model": ["4"] # malformed link: length 1
}
}
}
errors = list(Draft7Validator(schema).iter_errors(bad_prompt))
print(f"error_count={len(errors)}")
for e in errors:
print("-", e.message)
PYRepository: Comfy-Org/ComfyUI
Length of output: 74
🏁 Script executed:
git diff HEAD -- schemas/prompt.json | head -100Repository: Comfy-Org/ComfyUI
Length of output: 43
🏁 Script executed:
git status schemas/prompt.json && echo "---" && git diff --cached HEAD -- schemas/prompt.json | head -100Repository: Comfy-Org/ComfyUI
Length of output: 130
🏁 Script executed:
# Check the actual file content and git log for this file
cat schemas/prompt.json | head -60 && echo "---" && git log --oneline -10 -- schemas/prompt.jsonRepository: Comfy-Org/ComfyUI
Length of output: 2032
🏁 Script executed:
# Check the previous state of the file before the latest commit
git show HEAD~1:schemas/prompt.json 2>/dev/null | grep -A 5 '"inputs"' || echo "Not found in previous commit"Repository: Comfy-Org/ComfyUI
Length of output: 89
🏁 Script executed:
# Check if schemas/prompt.json was newly added in this PR
git log --oneline --follow -- schemas/prompt.json | tail -5 && echo "---" && git show --name-status HEAD | grep prompt.jsonRepository: Comfy-Org/ComfyUI
Length of output: 142
node_link is not enforced for inputs and outputs values.
With additionalProperties: true, the schema accepts malformed arrays like ["4"], allowing payloads that pass validation but fail at runtime. The node_link definition requires exactly 2 items but is never referenced.
Suggested schema fix
"inputs": {
"type": "object",
"description": "Node inputs. Keys are parameter names. Values are either direct values or node links [source_id, output_index].",
- "additionalProperties": true
+ "additionalProperties": {
+ "anyOf": [
+ { "$ref": "#/definitions/node_link" },
+ { "not": { "type": "array" } }
+ ]
+ }
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@schemas/prompt.json` around lines 38 - 42, The schema currently lets any
value for "inputs" (and likewise "outputs") via additionalProperties: true, so
arrays like ["4"] slip through; update the "inputs" and "outputs" property
schemas to replace additionalProperties: true with an object schema that uses
additionalProperties: { anyOf: [ { }, { $ref: "#/definitions/node_link" } ] }
(or equivalent) so each property value is either any valid JSON value or a
reference to the existing "node_link" definition (which enforces exactly two
items); ensure the change is applied to both "inputs" and "outputs" entries and
that the "node_link" definition name is used verbatim.
History items store prompt data as (output, inputs, extra_data, ...) tuple/list format, not as a dict. The sanitizer now handles both formats to properly strip SENSITIVE_EXTRA_DATA_KEYS. Fixes coderabbit Critical review on Comfy-Org#13095
- Remove strict node ID regex, allow any string key - Change node additionalProperties to true Addresses coderabbit Major review on Comfy-Org#13095
|
👋 Hi! Just a friendly ping on this PR. This is a bounty task submission. Would appreciate your review when you have a moment! 🙏 If there are any changes needed, please let me know. Thanks! 🌶️ |
1 participant
Closes #8889
Summary
Adds sanitization to
execution.py:get_history()that stripsauth_token_comfy_organdapi_key_comfy_orgfrom prompt inputs in history items before returning them.Why
If history items are ever persisted to disk or accessed over open networks, these tokens could be exposed. This is a defensive measure.
Changes
_sanitize_history_item()method that deep-copies and strips sensitive keys from prompt inputsget_history()when no custommap_functionis providedauth_token_comfy_org,api_key_comfy_org