fix(security): run trusted server code on unauthenticated public flow builds#13540
fix(security): run trusted server code on unauthenticated public flow builds#13540erichare wants to merge 4 commits into
Conversation
… builds
The unauthenticated public build path (POST /api/v1/build_public_tmp/{flow_id}/flow)
builds a public flow as its owner and executes its components, running each node's
stored `code` via eval_custom_component_code. The custom-component hash gate
(validate_flow_for_current_settings) is a no-op under the default
allow_custom_components=true, so a public flow containing a plain CustomComponent —
or any node carrying arbitrary code — would execute on that path without
authentication (follow-up to H1-3754930).
Enforcing the hash gate on the public path is not viable: it rejects legitimate
flows whose built-in component code has merely drifted across versions ("outdated"),
which would break the public playground on every upgrade.
Instead, on the public path substitute the server's trusted code for each known
component type and reject nodes whose type is not a known server component:
- known type -> stored code replaced with the server's current code, so version
drift cannot break the build and a relabelled node cannot smuggle in its bytes;
- unknown/custom type carrying code -> rejected (400);
- codeless nodes (group/note) untouched; inlined sub-flows handled recursively.
The build then runs from this server-sanitized data. Operators who knowingly want
public flows to run custom component code can restore the prior DB-loaded build with
LANGFLOW_ALLOW_PUBLIC_CUSTOM_COMPONENTS=true (default false).
Authenticated builds are unaffected.
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR adds server-side code sanitization for unauthenticated public flow builds. Public flows now have their component code replaced with server-trusted templates to prevent execution of user-provided code by anonymous users. A new settings flag controls opt-in behavior for custom components, and the endpoint rejects flows containing unknown component types. ChangesPublic Flow Sanitization and Component Validation
🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 8 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (8 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
✅ Test Coverage AdvisorNo source changes detected without accompanying tests. Thanks for keeping coverage up! 🎉
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/lfx/tests/unit/utils/test_flow_validation.py (1)
115-149: ⚡ Quick winAdd a regression test for malformed non-string
component_typevalues.Current tests don’t pin behavior when
node.data.typeis unhashable (e.g., dict). Add a case that assertsprepare_public_flow_buildrejects it viaCustomComponentValidationError(not a rawTypeError).Suggested test shape
+@pytest.mark.asyncio +async def test_prepare_public_flow_build_rejects_malformed_component_type(monkeypatch): + from lfx.utils import flow_validation as fv + + monkeypatch.setattr("lfx.services.deps.get_settings_service", lambda: _public_settings()) + monkeypatch.setattr(fv, "_ensure_component_code_lookups", AsyncMock(return_value={"ChatInput": "# trusted"})) + + flow = { + "nodes": [ + { + "id": "x", + "data": { + "id": "x", + "type": {"bad": "type"}, + "node": {"display_name": "BadType", "template": {"code": {"value": "print(1)"}}}, + }, + } + ], + "edges": [], + } + with pytest.raises(CustomComponentValidationError): + await fv.prepare_public_flow_build(flow)As per coding guidelines, tests should include edge cases and error conditions for comprehensive coverage.
Also applies to: 169-244
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lfx/tests/unit/utils/test_flow_validation.py` around lines 115 - 149, Add a regression test that constructs a node whose component_type / node.data.type is a non-string/unhashable value (e.g., a dict) and assert that calling prepare_public_flow_build raises CustomComponentValidationError (not a raw TypeError); locate this alongside existing flow_validation tests (e.g., near test_substitute_trusted_node_code*), import prepare_public_flow_build and CustomComponentValidationError from lfx.utils.flow_validation, pass the malformed node into prepare_public_flow_build, and use an explicit assertRaises/pytest.raises for CustomComponentValidationError to confirm the correct error is raised.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lfx/src/lfx/utils/flow_validation.py`:
- Around line 297-301: The code checks component_type for truthiness then does
"component_type in type_to_code" which can raise TypeError for
unhashable/non-string values; update the branch that currently uses
node_data.get("type") so you first verify isinstance(component_type, str) (or
otherwise hashable) before testing membership in type_to_code (and treat
non-string types as invalid/missing so they follow the existing policy error
path), referencing the variables component_type, node_data, type_to_code and
code_field to find and fix the block.
---
Nitpick comments:
In `@src/lfx/tests/unit/utils/test_flow_validation.py`:
- Around line 115-149: Add a regression test that constructs a node whose
component_type / node.data.type is a non-string/unhashable value (e.g., a dict)
and assert that calling prepare_public_flow_build raises
CustomComponentValidationError (not a raw TypeError); locate this alongside
existing flow_validation tests (e.g., near test_substitute_trusted_node_code*),
import prepare_public_flow_build and CustomComponentValidationError from
lfx.utils.flow_validation, pass the malformed node into
prepare_public_flow_build, and use an explicit assertRaises/pytest.raises for
CustomComponentValidationError to confirm the correct error is raised.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7c2b27d2-cbe4-4f06-a51f-f150285a28fe
📒 Files selected for processing (5)
src/backend/base/langflow/api/v1/chat.pysrc/backend/tests/unit/test_chat_endpoint.pysrc/lfx/src/lfx/services/settings/base.pysrc/lfx/src/lfx/utils/flow_validation.pysrc/lfx/tests/unit/utils/test_flow_validation.py
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release-1.10.1 #13540 +/- ##
=================================================
Coverage ? 58.48%
=================================================
Files ? 2302
Lines ? 219245
Branches ? 32951
=================================================
Hits ? 128220
Misses ? 89560
Partials ? 1465
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Combine the two complementary public-flow hardening defenses (report H1-3754930) that landed on separate branches: - prepare_public_flow_build (this PR): substitutes the server's trusted code into every known component and rejects unrecognized custom components on the unauthenticated public build path. - validate_public_flow_no_code_execution (release-1.10.1): blocks code-execution component types (Python REPL/Interpreter, legacy Python Code Structured tool, Smart Transform) and flow-invoking components (Run Flow, Sub Flow, Flow as Tool — the transitive case). Both now run on the public path in chat.py: the type/hash block first (fail-fast, unconditional), then trusted-code substitution. The allow_public_custom_components opt-in only widens custom-component trust; it does not re-open the code-execution/flow-invocation block. Conflict resolution notes: - settings/base.py was refactored into mixins on release-1.10.1; moved the new allow_public_custom_components field into the SecuritySettings group and updated its docstring to describe the substitution behavior. - flow_validation.py and test_flow_validation.py: kept both sides' added functions/tests (disjoint). Removed a duplicated blocked.append block left by an earlier web edit.
…ld list The public-flow trusted-code fix added the allow_public_custom_components setting to SecuritySettings, which bumped Settings.model_fields to 147 and tripped test_field_count_unchanged (147 != 146). Add the field to the curated EXPECTED_FIELDS set under a 1.10.1 section.
Summary
Follow-up to the H1-3754930 public-flow code-execution hardening. The unauthenticated public build path
POST /api/v1/build_public_tmp/{flow_id}/flowbuilds a public flow as its owner and executes its components — each node's storedcodeis run viaeval_custom_component_code. The custom-component hash gate (validate_flow_for_current_settings) is a no-op under the defaultallow_custom_components=true, so a public flow containing a plainCustomComponent— or any node carrying arbitrary code — would execute on that path without authentication.This was raised on the transitive-execution PR: the existing type-based blocks (PythonREPL/Smart Transform/RunFlow/…) don't cover arbitrary custom code, and
CustomComponentcan't be blocklisted by type (a relabelled node forges any type).Why not just enforce the hash gate on the public path
Enforcing the hash gate (allow-list of code matching current templates) rejects legitimate flows whose built-in component code has merely drifted across versions — the gate can't distinguish "old but legit built-in" from "arbitrary code in a known-type node" (both just fail to match the current hash). A standard chatbot flow is flagged
outdatedand blocked; this would break the public playground on every upgrade.The fix
On the public path, run the server's trusted code instead of trusting the node's stored bytes:
codeis replaced with the server's current code for that type. Version drift can no longer break the build, and a node relabelled to a known type cannot smuggle in its own bytes (the server's code runs).400 This flow cannot be executed.).The build then runs from this server-sanitized data (passed via the existing
dataparam; the caller never supplies it — it is derived from the stored flow). Authenticated builds are unaffected.Opt-out
Operators who knowingly want public flows to run custom component code can restore the prior DB-loaded build (which honors
allow_custom_components) withLANGFLOW_ALLOW_PUBLIC_CUSTOM_COMPONENTS=true(defaultfalse).Changes
lfx/utils/flow_validation.py:collect_component_code_lookups,_substitute_trusted_node_code,_ensure_component_code_lookups,prepare_public_flow_build.lfx/services/settings/base.py:allow_public_custom_components(defaultfalse).api/v1/chat.py: public route substitutes/blocks viaprepare_public_flow_buildand builds from the sanitized data.Test plan
src/lfx/tests/unit/utils/test_flow_validation.py(17 passed): trusted-code substitution, unknown-type block, relabelled-code neutralization, opt-in passthrough, recursion, fail-closed, empty/no-op.src/backend/tests/unit/test_chat_endpoint.py::test_build_public_tmp_rejects_custom_component(new) — unknown custom component on the public path → 400.*public_tmp*route suite (21 passed) — standard flows still build (substitution avoids theoutdatedbreak); no regression.ruff check/ruff formatclean.Notes
Independent of, and complementary to, the transitive RunFlow/SubFlow/FlowTool block on
fix/public-flow-code-exec-rce(both targetrelease-1.10.0; expect a trivial merge in thebuild_public_tmpvalidation block depending on merge order). PerSECURITY.md, vulnerability reports go through HackerOne — this PR is the remediation for an internally-tracked follow-up.Summary by CodeRabbit
New Features
Bug Fixes
Tests