Skip to content

fix(cli): reject credential-bearing Git dependencies - #8542

Open
John Kennedy (jkennedyvz) wants to merge 8 commits into
mainfrom
open-swe/jkb-55-safe-git-urls
Open

John Kennedy (jkennedyvz) wants to merge 8 commits into
mainfrom
open-swe/jkb-55-safe-git-urls

Conversation

@jkennedyvz

@jkennedyvz John Kennedy (jkennedyvz) commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Description

Reject Git HTTP dependency URLs containing userinfo before Docker generation so credentials cannot persist in Dockerfiles or image layers. Validation covers direct Python dependencies, local Python metadata, uv project and lock inputs, and common Node.js package and lock files at both workspace and application roots. Errors identify the inspected file path without echoing dependency contents.

Detection intentionally scans raw text conservatively, so placeholders such as ${GIT_TOKEN}@... and commented examples are rejected. Coverage remains best-effort because arbitrary files, nested requirements includes, and constraints can still be copied into a build context; the README now describes that boundary and warns against credentials in copied pip configuration.

Test Plan

  • Validate credential-bearing direct, local-manifest, uv-managed, Node.js, and workspace Git URLs are rejected without echoing credentials
  • Validate credential-free HTTPS and SSH Git URLs remain supported
  • Validate direct dependency errors identify the source langgraph.json
  • make format; make lint; focused config suite: 132 passed

Made by Open SWE

Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>
@jkennedyvz
John Kennedy (jkennedyvz) marked this pull request as ready for review August 5, 2026 22:17

@open-swe open-swe Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Open SWE Review found 1 potential issue.

Open in WebView Open SWE trace

Comment thread libs/cli/langgraph_cli/config.py Outdated
Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>
@jkennedyvz
John Kennedy (jkennedyvz) enabled auto-merge (squash) August 5, 2026 22:44

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good hardening — rejecting credential-bearing Git URLs at config-parse time is the correct place to stop them before they reach a generated Dockerfile or image layer. Since the check is regex based, cover the obvious bypasses in tests: percent-encoded userinfo and alternative schemes such as git+ssh where a username may be embedded differently. Confirming the error message points users at the secret-backed credential-helper flow keeps the escape hatch discoverable.

@eliornl Elior Nataf Lackritz (eliornl) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Superseded, see the inline comments on the diff.

Comment thread libs/cli/README.md Outdated
Comment thread libs/cli/langgraph_cli/config.py
Comment thread libs/cli/langgraph_cli/config.py
Comment thread libs/cli/langgraph_cli/config.py
Comment thread libs/cli/langgraph_cli/schemas.py
@eliornl

Copy link
Copy Markdown
Contributor

Title: fix(cli): reject credential-bearing Git dependencies. cli is an allowed scope and is the convention in this repo.

Gate is clean on my end: lint_package, lint_tests, 345 tests passing in libs/cli, and generate_schema.py produces no drift.

Specifics are inline on the diff.

@jkennedyvz John Kennedy (jkennedyvz) changed the title fix: reject credential-bearing Git dependencies fix(cli): reject credential-bearing Git dependencies Aug 11, 2026

@eliornl Elior Nataf Lackritz (eliornl) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this covers everything I raised. Verified against 62ecd541: Node package and lock files are now scanned, the README describes what is actually covered, and errors name the file without echoing the value. libs/cli clean locally, 352 passed.

One nit, not blocking: the validate_config call site still passes no source, so a credential in langgraph.json itself gets the generic message. Passing config_path would close it.

Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>
@open-swe

open-swe Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Followed up on the final approval nit in 0cce2d2: direct dependency validation now reports the source langgraph.json path without echoing credential contents. Focused config tests pass (132 passed), and all review threads are resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants