Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| "skopeo", | ||
| "copy", | ||
| *(["--src-username", "x-access-token", "--src-password", github_token] if github_token else []), | ||
| *(["--src-username", "x-access-token", "--src-password", github_token] if github_token and image.startswith("ghcr.io/") else []), |
Check failure
Code scanning / CodeQL
Incomplete URL substring sanitization High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 12 hours ago
General fix: Instead of treating image as an opaque string and using startswith("ghcr.io/"), normalize and parse the image reference to reliably identify GHCR images. Strip any known transport prefix (like docker://) and then check whether the first path segment (the “registry” component) is exactly "ghcr.io".
Specific best fix in this file:
- Define a small helper function near the top-level of
platform.py(where other helpers likely live) that returnsTrueif a given image reference is hosted on GHCR:- If the string starts with
"docker://"(the prefix we add later when constructing the Skopeo source), strip that prefix. - After stripping, split on
'/'and check if the first component equals"ghcr.io".
- If the string starts with
- Replace both uses of
image.startswith("ghcr.io/")in:- The argument list construction containing
--src-username/--src-password(line ~780). - The
envassignment forGITHUB_TOKEN(line ~785).
with calls to this helper, e.g.is_ghcr_image(image).
- The argument list construction containing
This preserves existing behavior for normal "ghcr.io/..." images and also correctly handles cases where the image string might already include a docker:// prefix, while avoiding raw substring assumptions about arbitrary positions in a URL-like string.
We only touch apps/agentstack-cli/src/agentstack_cli/commands/platform.py, adding one helper function and updating the two conditions that currently use image.startswith("ghcr.io/"). No new imports are needed.
| @@ -43,6 +43,23 @@ | ||
| configuration = Configuration() | ||
|
|
||
|
|
||
| def is_ghcr_image(image: str) -> bool: | ||
| """ | ||
| Return True if the given container image reference points to ghcr.io. | ||
|
|
||
| Handles both plain image refs like "ghcr.io/owner/image:tag" and | ||
| refs that may include a "docker://" prefix, such as "docker://ghcr.io/owner/image:tag". | ||
| """ | ||
| # Strip a docker transport prefix if present. | ||
| if image.startswith("docker://"): | ||
| stripped = image[len("docker://") :] | ||
| else: | ||
| stripped = image | ||
| # The registry is the first path component. | ||
| registry = stripped.split("/", 1)[0] | ||
| return registry == "ghcr.io" | ||
|
|
||
|
|
||
| @functools.cache | ||
| def detect_driver() -> typing.Literal["lima", "wsl"]: | ||
| has_lima = (importlib.resources.files("agentstack_cli") / "data" / "limactl").is_file() or shutil.which("limactl") | ||
| @@ -777,12 +794,12 @@ | ||
| else [ | ||
| "skopeo", | ||
| "copy", | ||
| *(["--src-username", "x-access-token", "--src-password", github_token] if github_token and image.startswith("ghcr.io/") else []), | ||
| *(["--src-username", "x-access-token", "--src-password", github_token] if github_token and is_ghcr_image(image) else []), | ||
| f"docker://{image}", | ||
| f"containers-storage:{image}", | ||
| ], | ||
| f"Pulling image {image}", | ||
| env={"GITHUB_TOKEN": github_token} if github_token and image.startswith("ghcr.io/") else None, | ||
| env={"GITHUB_TOKEN": github_token} if github_token and is_ghcr_image(image) else None, | ||
| ) | ||
|
|
||
| # --- Kagenti platform installation --- |
| ], | ||
| f"Pulling image {image}", | ||
| env={"GITHUB_TOKEN": github_token} if github_token else None, | ||
| env={"GITHUB_TOKEN": github_token} if github_token and image.startswith("ghcr.io/") else None, |
Check failure
Code scanning / CodeQL
Incomplete URL substring sanitization High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 12 hours ago
In general, to fix incomplete URL or host substring checks, you should parse the value into its structured components (such as with urllib.parse.urlparse for URLs) and then compare only the relevant field (like hostname) against an allowlist, rather than searching or prefix-matching in the raw string.
Here, we’re not actually working with full URLs but with Docker/OCI image references: [registry/][namespace/]name[:tag][@digest]. The goal is to determine whether the registry domain is ghcr.io before sending GitHub Container Registry credentials. The safest way to preserve functionality and remove the substring concern is to normalize the image reference and extract the “registry” portion: if the string, when split on '/', has more than one segment and the first segment is exactly 'ghcr.io', then the registry is ghcr.io. This avoids accidental matches where ghcr.io appears later in the path or inside another hostname. We should encapsulate this logic in a small helper function inside the same file, e.g. _is_ghcr_image(image: str) -> bool, and then replace both uses of image.startswith("ghcr.io/") on lines 780 and 785 with this helper. No external dependencies are needed; we use simple string splitting.
Concretely:
- In
apps/agentstack-cli/src/agentstack_cli/commands/platform.py, define a helper function (near other helpers, or just before the code using it) that returnsTrueiff the image’s registry portion is exactly'ghcr.io'. This means: splitimageon'/'; if there is more than one part and the first part is'ghcr.io', returnTrue; otherwiseFalse. (If no registry is present, Docker assumesdocker.io, so we must not treat that asghcr.io.) - Replace the inline condition
github_token and image.startswith("ghcr.io/")in the list forskopeoarguments (line 780) and in theenv={...}expression (line 785) withgithub_token and _is_ghcr_image(image). - No imports or other definitions are required beyond the helper.
This preserves existing behavior for valid ghcr.io/... images while ensuring we don’t mistakenly treat arbitrary strings containing ghcr.io/ as belonging to that registry.
| @@ -767,6 +767,18 @@ | ||
| ) | ||
| finally: | ||
| await anyio.Path(host_path).unlink(missing_ok=True) | ||
|
|
||
| def _is_ghcr_image(image: str) -> bool: | ||
| """ | ||
| Return True if the image reference uses ghcr.io as its registry. | ||
| Docker/OCI image references are of the form: | ||
| [registry/][namespace/]name[:tag][@digest] | ||
| We consider the first path component to be the registry iff there is more | ||
| than one component and the first component is exactly "ghcr.io". | ||
| """ | ||
| parts = image.split("/") | ||
| return len(parts) > 1 and parts[0] == "ghcr.io" | ||
|
|
||
| if image_pull_mode in {ImagePullMode.guest, ImagePullMode.hybrid}: | ||
| github_token = get_local_github_token() | ||
| for image in loaded_images - images_to_import_from_host: | ||
| @@ -777,12 +789,21 @@ | ||
| else [ | ||
| "skopeo", | ||
| "copy", | ||
| *(["--src-username", "x-access-token", "--src-password", github_token] if github_token and image.startswith("ghcr.io/") else []), | ||
| *( | ||
| [ | ||
| "--src-username", | ||
| "x-access-token", | ||
| "--src-password", | ||
| github_token, | ||
| ] | ||
| if github_token and _is_ghcr_image(image) | ||
| else [] | ||
| ), | ||
| f"docker://{image}", | ||
| f"containers-storage:{image}", | ||
| ], | ||
| f"Pulling image {image}", | ||
| env={"GITHUB_TOKEN": github_token} if github_token and image.startswith("ghcr.io/") else None, | ||
| env={"GITHUB_TOKEN": github_token} if github_token and _is_ghcr_image(image) else None, | ||
| ) | ||
|
|
||
| # --- Kagenti platform installation --- |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant architectural shift by integrating Kagenti into the Agent Stack CLI. This integration simplifies agent management, enhances scalability, and provides a more streamlined local development experience. The changes involve removing custom Kubernetes management components and adopting Kagenti's deployment and discovery mechanisms. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a significant refactoring to integrate kagenti for agent lifecycle management, replacing the custom Kubernetes provider deployment and build system. The changes are extensive, touching many parts of the codebase from the CLI to the server-side services and database models. Key changes include removing the build command, simplifying the add and update agent commands, and introducing a new kagenti client for agent discovery. The provider model has been greatly simplified, and a new database migration reflects these changes. Overall, the changes are consistent with the goal of delegating agent management to kagenti. I've identified a few areas where maintainability could be improved by reducing hardcoded values in the platform setup scripts.
Note: Security Review did not run due to the size of the PR.
| await run_in_vm( | ||
| vm_name, | ||
| [ | ||
| "bash", | ||
| "-c", | ||
| textwrap.dedent("""\ | ||
| kubectl --kubeconfig=/kubeconfig apply -f https://github.com/kubernetes-sigs/gateway-api/releases/download/v1.4.0/standard-install.yaml | ||
| kubectl --kubeconfig=/kubeconfig apply -f https://github.com/cert-manager/cert-manager/releases/download/v1.17.2/cert-manager.yaml | ||
| kubectl --kubeconfig=/kubeconfig wait --for=condition=Available deployment -n cert-manager cert-manager-webhook --timeout=120s | ||
| """), | ||
| ], | ||
| "Installing kagenti prerequisites (Gateway API CRDs, cert-manager)", |
There was a problem hiding this comment.
The URLs for installing Gateway API CRDs and cert-manager are hardcoded with specific versions (v1.4.0 and v1.17.2 respectively). While this ensures reproducibility, it makes updates require code changes. To improve maintainability, consider defining these versions as constants at the top of the file.
| await run_in_vm( | ||
| vm_name, | ||
| [ | ||
| "bash", | ||
| "-c", | ||
| textwrap.dedent("""\ | ||
| ISTIO_VERSION=1.28.0 | ||
| ISTIO_REPO=https://istio-release.storage.googleapis.com/charts/ | ||
| helm repo add istio "$ISTIO_REPO" 2>/dev/null || true | ||
| helm repo update istio | ||
| kubectl --kubeconfig=/kubeconfig create namespace istio-system --dry-run=client -o yaml | kubectl --kubeconfig=/kubeconfig apply -f - | ||
| helm upgrade --install istio-base istio/base --version=$ISTIO_VERSION --namespace=istio-system --kubeconfig=/kubeconfig --wait --force-conflicts | ||
| helm upgrade --install istiod istio/istiod --version=$ISTIO_VERSION --namespace=istio-system --kubeconfig=/kubeconfig --wait --force-conflicts \ | ||
| --set pilot.resources.requests.cpu=50m \ | ||
| --set pilot.resources.requests.memory=256Mi | ||
| """), | ||
| ], | ||
| "Installing Istio (Gateway API controller)", |
There was a problem hiding this comment.
The Istio version (1.28.0) is hardcoded within this shell script block. This could lead to maintenance issues if kagenti's Istio dependency changes in the future. It would be more maintainable to define this version as a constant at the top of the file, making it easier to update.
| agent_namespace = agent.get("namespace", namespace) | ||
|
|
||
| # Construct service URL from k8s naming convention (service port 8080) | ||
| url = f"http://{name}.{agent_namespace}.svc.cluster.local:8080" |
There was a problem hiding this comment.
The agent service URL is constructed with a hardcoded port 8080. If kagenti allows agents to run on different ports, this could cause connection issues. If this is a fixed convention from kagenti, adding a comment to clarify this would be helpful. For more robustness, consider if the port can be discovered from the service definition rather than being hardcoded.
jezekra1
force-pushed
the
poc/kagenti-integration
branch
from
7de860d to
9c0fd63
Compare
Signed-off-by: Radek Ježek <radek.jezek@ibm.com>
jezekra1
force-pushed
the
poc/kagenti-integration
branch
from
9c0fd63 to
4ee4c89
Compare
1 participant
Signed-off-by: Radek Ježek radek.jezek@ibm.com
Summary
Linked Issues
Documentation