From 48ecb3bc816853fa3cd22fbb65ce2e8973997b6b Mon Sep 17 00:00:00 2001 From: John Kennedy <65985482+jkennedyvz@users.noreply.github.com> Date: Tue, 11 Aug 2026 09:40:13 -0700 Subject: [PATCH] fix(cli): address Git dependency review feedback --- libs/cli/README.md | 2 +- libs/cli/langgraph_cli/config.py | 28 ++++++-- libs/cli/langgraph_cli/schemas.py | 3 +- libs/cli/schemas/schema.json | 6 +- libs/cli/schemas/schema.v0.json | 6 +- libs/cli/tests/unit_tests/test_config.py | 92 ++++++++++++++++++++++++ 6 files changed, 123 insertions(+), 14 deletions(-) diff --git a/libs/cli/README.md b/libs/cli/README.md index 82016c0b9..9f33b561a 100644 --- a/libs/cli/README.md +++ b/libs/cli/README.md @@ -103,7 +103,7 @@ The CLI uses a `langgraph.json` configuration file with these key settings: } ``` -Git dependencies must use credential-free URLs. The CLI rejects HTTP Git URLs with userinfo because generated Dockerfiles and image layers can retain embedded usernames or tokens. For private dependencies, provide short-lived credentials through your build environment's secret-backed Git credential helper. Do not store credentials in `langgraph.json`, requirement or lock files, or a `pip_config_file` copied into the image. +Git dependencies should use credential-free URLs. The CLI conservatively scans direct `langgraph.json` dependencies, common Python package files, uv project and lock files, and common Node.js package and lock files for HTTP Git URLs with userinfo. This check is not exhaustive: generated Docker builds can copy other files, including nested requirement or constraint files, into image layers without scanning them. For private dependencies, provide short-lived credentials through your build environment's secret-backed Git credential helper. Do not store credentials in copied files such as `langgraph.json` or `pip_config_file`. See the [full documentation](https://reference.langchain.com/python/langgraph-cli) for detailed configuration options. diff --git a/libs/cli/langgraph_cli/config.py b/libs/cli/langgraph_cli/config.py index aabc824d7..5fdfc6cfb 100644 --- a/libs/cli/langgraph_cli/config.py +++ b/libs/cli/langgraph_cli/config.py @@ -92,32 +92,37 @@ def _has_git_http_url_userinfo(dependency: str) -> bool: ) -def _validate_git_http_url_userinfo(values: Iterable[str]) -> None: +def _validate_git_http_url_userinfo( + values: Iterable[str], *, source: pathlib.Path | None = None +) -> None: """Reject credential-bearing Git HTTP URLs without echoing their values.""" if not any(_has_git_http_url_userinfo(value) for value in values): return - raise click.UsageError( + message = ( "Git dependency URLs must not contain credentials or other URL " "userinfo because generated Dockerfiles and image layers can retain " "them. Use a credential-free Git URL and provide short-lived " "credentials through your build environment's secret-backed Git " "credential helper." ) + if source is not None: + message += f" Found in: {source}" + raise click.UsageError(message) def _validate_git_http_url_userinfo_files(paths: Iterable[pathlib.Path]) -> None: """Reject credential-bearing Git HTTP URLs in dependency files.""" - contents: list[str] = [] for path in paths: + path = path.resolve() if not path.is_file(): continue try: - contents.append(path.read_text(encoding="utf-8", errors="replace")) + contents = path.read_text(encoding="utf-8", errors="replace") except OSError: raise click.UsageError( f"Could not inspect dependency file for embedded credentials: {path}" ) from None - _validate_git_http_url_userinfo(contents) + _validate_git_http_url_userinfo([contents], source=path) def _validate_local_dependency_files(config_path: pathlib.Path, config: Config) -> None: @@ -1553,7 +1558,18 @@ def node_config_to_docker( ) -> tuple[str, dict[str, str]]: # Calculate paths for monorepo support install_root = ( - pathlib.Path(build_context).resolve() if build_context else config_path.parent + pathlib.Path(build_context).resolve() + if build_context + else config_path.parent.resolve() + ) + config_root = config_path.parent.resolve() + dependency_roots = ( + (install_root, config_root) if install_root != config_root else (install_root,) + ) + _validate_git_http_url_userinfo_files( + root / name + for root in dependency_roots + for name in ("package.json", "package-lock.json", "yarn.lock", "pnpm-lock.yaml") ) install_cmd = install_command or _get_node_pm_install_cmd(install_root) if build_context: diff --git a/libs/cli/langgraph_cli/schemas.py b/libs/cli/langgraph_cli/schemas.py index 6678d7d67..1d527a94e 100644 --- a/libs/cli/langgraph_cli/schemas.py +++ b/libs/cli/langgraph_cli/schemas.py @@ -650,7 +650,8 @@ class Config(TypedDict, total=False): pip_config_file: str | None """Optional. Path to a pip config file (e.g., "/etc/pip.conf" or "pip.ini") for controlling - package installation (custom indices, credentials, etc.). + package installation (custom indices, timeouts, etc.). The file is copied into the + generated image, so it must not contain credentials or other secrets. Only relevant if Python dependencies are installed via pip. If omitted, default pip settings are used. """ diff --git a/libs/cli/schemas/schema.json b/libs/cli/schemas/schema.json index 6eec6205b..91dfad2f2 100644 --- a/libs/cli/schemas/schema.json +++ b/libs/cli/schemas/schema.json @@ -28,7 +28,7 @@ "type": "null" } ], - "description": "Optional. Path to a pip config file (e.g., \"/etc/pip.conf\" or \"pip.ini\") for controlling\npackage installation (custom indices, credentials, etc.).\n\nOnly relevant if Python dependencies are installed via pip. If omitted, default pip settings are used.\n" + "description": "Optional. Path to a pip config file (e.g., \"/etc/pip.conf\" or \"pip.ini\") for controlling\npackage installation (custom indices, timeouts, etc.). The file is copied into the\ngenerated image, so it must not contain credentials or other secrets.\n\nOnly relevant if Python dependencies are installed via pip. If omitted, default pip settings are used.\n" }, "_INTERNAL_docker_tag": { "anyOf": [ @@ -270,7 +270,7 @@ "type": "null" } ], - "description": "Optional. Path to a pip config file (e.g., \"/etc/pip.conf\" or \"pip.ini\") for controlling\npackage installation (custom indices, credentials, etc.).\n\nOnly relevant if Python dependencies are installed via pip. If omitted, default pip settings are used.\n" + "description": "Optional. Path to a pip config file (e.g., \"/etc/pip.conf\" or \"pip.ini\") for controlling\npackage installation (custom indices, timeouts, etc.). The file is copied into the\ngenerated image, so it must not contain credentials or other secrets.\n\nOnly relevant if Python dependencies are installed via pip. If omitted, default pip settings are used.\n" }, "_INTERNAL_docker_tag": { "anyOf": [ @@ -1346,4 +1346,4 @@ "title": "LangGraph CLI Configuration", "description": "Configuration schema for langgraph-cli", "version": "v0" -} \ No newline at end of file +} diff --git a/libs/cli/schemas/schema.v0.json b/libs/cli/schemas/schema.v0.json index 6eec6205b..91dfad2f2 100644 --- a/libs/cli/schemas/schema.v0.json +++ b/libs/cli/schemas/schema.v0.json @@ -28,7 +28,7 @@ "type": "null" } ], - "description": "Optional. Path to a pip config file (e.g., \"/etc/pip.conf\" or \"pip.ini\") for controlling\npackage installation (custom indices, credentials, etc.).\n\nOnly relevant if Python dependencies are installed via pip. If omitted, default pip settings are used.\n" + "description": "Optional. Path to a pip config file (e.g., \"/etc/pip.conf\" or \"pip.ini\") for controlling\npackage installation (custom indices, timeouts, etc.). The file is copied into the\ngenerated image, so it must not contain credentials or other secrets.\n\nOnly relevant if Python dependencies are installed via pip. If omitted, default pip settings are used.\n" }, "_INTERNAL_docker_tag": { "anyOf": [ @@ -270,7 +270,7 @@ "type": "null" } ], - "description": "Optional. Path to a pip config file (e.g., \"/etc/pip.conf\" or \"pip.ini\") for controlling\npackage installation (custom indices, credentials, etc.).\n\nOnly relevant if Python dependencies are installed via pip. If omitted, default pip settings are used.\n" + "description": "Optional. Path to a pip config file (e.g., \"/etc/pip.conf\" or \"pip.ini\") for controlling\npackage installation (custom indices, timeouts, etc.). The file is copied into the\ngenerated image, so it must not contain credentials or other secrets.\n\nOnly relevant if Python dependencies are installed via pip. If omitted, default pip settings are used.\n" }, "_INTERNAL_docker_tag": { "anyOf": [ @@ -1346,4 +1346,4 @@ "title": "LangGraph CLI Configuration", "description": "Configuration schema for langgraph-cli", "version": "v0" -} \ No newline at end of file +} diff --git a/libs/cli/tests/unit_tests/test_config.py b/libs/cli/tests/unit_tests/test_config.py index 1e0be96d8..bcd8c8c1c 100644 --- a/libs/cli/tests/unit_tests/test_config.py +++ b/libs/cli/tests/unit_tests/test_config.py @@ -261,6 +261,7 @@ def test_validate_config(): "git+https://user:secret-token@github.com/org/private.git@main", "private-package @ git+http://token@github.com/org/private.git", "git+HTTPS://user%40example.com:secret%2Ftoken@github.com/org/private.git", + "git+https://${GIT_TOKEN}@github.com/org/private.git", ], ) def test_validate_config_rejects_git_http_url_userinfo(dependency: str): @@ -279,6 +280,96 @@ def test_validate_config_rejects_git_http_url_userinfo(dependency: str): assert "secret%2Ftoken" not in message +@pytest.mark.parametrize( + "manifest", ["package.json", "package-lock.json", "yarn.lock", "pnpm-lock.yaml"] +) +def test_config_to_docker_rejects_git_http_url_userinfo_in_node_files( + tmp_path: pathlib.Path, manifest: str +): + config_path = tmp_path / "langgraph.json" + config_path.write_text("{}\n") + (tmp_path / "agent.js").write_text("export const graph = {};\n") + (tmp_path / "package.json").write_text('{"name":"agent"}\n') + (tmp_path / manifest).write_text( + '"priv": "git+https://user:secret-token@github.com/org/private.git"\n' + ) + config = validate_config( + { + "node_version": "20", + "graphs": {"agent": "./agent.js:graph"}, + } + ) + + with pytest.raises(click.UsageError) as exc_info: + config_to_docker( + config_path, + config, + base_image="langchain/langgraphjs-api", + ) + + message = str(exc_info.value) + assert "must not contain credentials or other URL userinfo" in message + assert "secret-token" not in message + assert f"Found in: {(tmp_path / manifest).resolve()}" in message + + +def test_config_to_docker_allows_node_git_urls_without_http_userinfo( + tmp_path: pathlib.Path, +): + config_path = tmp_path / "langgraph.json" + config_path.write_text("{}\n") + (tmp_path / "agent.js").write_text("export const graph = {};\n") + (tmp_path / "package.json").write_text( + '{"dependencies":{"public":"git+https://github.com/org/public.git"}}\n' + ) + config = validate_config( + { + "node_version": "20", + "graphs": {"agent": "./agent.js:graph"}, + } + ) + + docker, _ = config_to_docker( + config_path, + config, + base_image="langchain/langgraphjs-api", + ) + + assert f"ADD . /deps/{tmp_path.name}" in docker + + +def test_config_to_docker_rejects_git_http_url_userinfo_in_node_workspace( + tmp_path: pathlib.Path, +): + config_root = tmp_path / "apps" / "agent" + config_root.mkdir(parents=True) + config_path = config_root / "langgraph.json" + config_path.write_text("{}\n") + (config_root / "agent.js").write_text("export const graph = {};\n") + (config_root / "package.json").write_text( + '{"dependencies":{"priv":"git+https://secret-token@github.com/org/private.git"}}\n' + ) + (tmp_path / "package.json").write_text('{"name":"workspace"}\n') + config = validate_config( + { + "node_version": "20", + "graphs": {"agent": "./agent.js:graph"}, + } + ) + + with pytest.raises(click.UsageError) as exc_info: + config_to_docker( + config_path, + config, + base_image="langchain/langgraphjs-api", + build_context=str(tmp_path), + ) + + message = str(exc_info.value) + assert "secret-token" not in message + assert f"Found in: {(config_root / 'package.json').resolve()}" in message + + @pytest.mark.parametrize( "dependency", [ @@ -326,6 +417,7 @@ def test_config_to_docker_rejects_git_http_url_userinfo_in_requirements( message = str(exc_info.value) assert "must not contain credentials or other URL userinfo" in message assert "secret-token" not in message + assert f"Found in: {(tmp_path / 'requirements.txt').resolve()}" in message @pytest.mark.parametrize("manifest", ["pyproject.toml", "uv.lock"])