docs(kartograph): add ArgoCD PKCE login skill + Agent Sandbox CRD debugging notes#796
Open
aredenba-rh wants to merge 1 commit into
Open
docs(kartograph): add ArgoCD PKCE login skill + Agent Sandbox CRD debugging notes#796aredenba-rh wants to merge 1 commit into
aredenba-rh wants to merge 1 commit into
Conversation
…ugging notes Adds the kartograph Cursor skill (ArgoCD SSO workaround via manual PKCE login against Dex, plus common REST API endpoints/gotchas) and a new section documenting the Agent Sandbox CRD (agents.x-k8s.io) as an unowned cluster-side dependency: how to check its live presence via ArgoCD's cluster API-discovery cache, why RBAC syncing "Healthy" is not evidence the CRD exists, and that no GitOps repo we have access to (hp-fleet-gitops, hybrid-platforms-gitops/infrastructure, ambient-code-gitops) installs or owns it. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a Kartograph Cursor skill covering repository, GitOps, ArgoCD, Agent Sandbox, Vault, and Konflux operations. Adds an executable Python utility that performs Dex OAuth2/PKCE authentication, receives the local callback, validates CSRF state, exchanges the authorization code, and writes the resulting ArgoCD ID token. Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant argocd_pkce_login.py
participant Browser
participant Dex
participant ArgoCD API
Operator->>argocd_pkce_login.py: Run login script
argocd_pkce_login.py->>Browser: Display Dex authorization URL
Browser->>Dex: Complete authorization
Dex->>argocd_pkce_login.py: Send callback code and state
argocd_pkce_login.py->>Dex: Exchange code with PKCE verifier
Dex-->>argocd_pkce_login.py: Return id_token
argocd_pkce_login.py->>ArgoCD API: Use token for operational requests
🚥 Pre-merge checks | ✅ 9 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (9 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.cursor/skills/kartograph/SKILL.md (1)
22-27: 🔒 Security & Privacy | 🔵 TrivialDocuments bypassing PR review via direct push to unprotected
main.Encouraging direct pushes to
hp-fleet-gitopsmainas a "faster path" formalizes a review-bypass workflow. This reflects that repo's existing (lack of) branch protection rather than something introduced here, but it's worth flagging as a standing security-posture gap for the GitOps repo (no required review before manifests reach the auto-syncing ArgoCD Application).🤖 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 @.cursor/skills/kartograph/SKILL.md around lines 22 - 27, Revise the hp-fleet-gitops guidance to remove the recommendation that urgent fixes may be pushed directly to the unprotected main branch. Keep the automated update-deploy-tag workflow and ArgoCD synchronization details, and direct users to the PR-based process rather than documenting a review bypass.
🤖 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 @.cursor/skills/kartograph/scripts/argocd_pkce_login.py:
- Around line 47-73: Update wait_for_callback so the 180-second timeout is an
actual overall deadline rather than a per-request timeout. Track a monotonic
deadline before the request loop, recompute the remaining time before each
server.handle_request() call, and exit or raise the established timeout error
when no callback arrives by the deadline; preserve immediate completion when
code or error is received.
- Around line 118-121: Update the token-writing flow around id_token and
args.out to create the output file with owner-only permissions (0600) at
creation time, rather than relying on the process umask. Preserve writing the
token and reporting its length, and ensure existing files are not left with
broader permissions.
- Around line 103-116: Add a finite timeout to the urllib.request.urlopen call
in the Dex token exchange so stalled Dex responses cannot block the login flow
indefinitely. Keep the existing request construction and JSON token parsing
unchanged.
---
Nitpick comments:
In @.cursor/skills/kartograph/SKILL.md:
- Around line 22-27: Revise the hp-fleet-gitops guidance to remove the
recommendation that urgent fixes may be pushed directly to the unprotected main
branch. Keep the automated update-deploy-tag workflow and ArgoCD synchronization
details, and direct users to the PR-based process rather than documenting a
review bypass.
🪄 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: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: f48cdb6d-74ff-473d-938f-4c35b1be43bc
📒 Files selected for processing (2)
.cursor/skills/kartograph/SKILL.md.cursor/skills/kartograph/scripts/argocd_pkce_login.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift-hyperfleet/architecture(manual)openshift-hyperfleet/hyperfleet-api(manual)openshift-hyperfleet/hyperfleet-sentinel(manual)openshift-hyperfleet/hyperfleet-adapter(manual)openshift-hyperfleet/hyperfleet-broker(manual)
Comment on lines
+47
to
+73
| def wait_for_callback() -> dict[str, str | None]: | ||
| result: dict[str, str | None] = {} | ||
|
|
||
| class Handler(http.server.BaseHTTPRequestHandler): | ||
| def do_GET(self) -> None: | ||
| parsed = urllib.parse.urlparse(self.path) | ||
| if parsed.path != CALLBACK_PATH: | ||
| self.send_response(404) | ||
| self.end_headers() | ||
| return | ||
| qs = urllib.parse.parse_qs(parsed.query) | ||
| result["code"] = qs.get("code", [None])[0] | ||
| result["state"] = qs.get("state", [None])[0] | ||
| result["error"] = qs.get("error", [None])[0] | ||
| self.send_response(200) | ||
| self.send_header("Content-Type", "text/html") | ||
| self.end_headers() | ||
| self.wfile.write(b"<html><body>Login captured, you can close this tab.</body></html>") | ||
|
|
||
| def log_message(self, *_args: object) -> None: | ||
| pass | ||
|
|
||
| server = http.server.HTTPServer(("127.0.0.1", CALLBACK_PORT), Handler) | ||
| server.timeout = 180 | ||
| while "code" not in result and "error" not in result: | ||
| server.handle_request() | ||
| return result |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
server.timeout doesn't bound the wait — loop retries forever (CWE-835-adjacent hang).
server.timeout = 180 only caps a single handle_request() call. If it expires with no request received, handle_timeout() is a no-op and the while condition is still true, so the loop just calls handle_request() again — indefinitely. This is a blocking wait with no actual deadline, contrary to what the 180s value implies.
🔒️ Proposed fix: enforce an actual deadline
+import time
+
def wait_for_callback() -> dict[str, str | None]:
result: dict[str, str | None] = {}
...
server = http.server.HTTPServer(("127.0.0.1", CALLBACK_PORT), Handler)
server.timeout = 180
- while "code" not in result and "error" not in result:
- server.handle_request()
+ deadline = time.monotonic() + 180
+ while "code" not in result and "error" not in result:
+ if time.monotonic() > deadline:
+ raise SystemExit("Timed out waiting for SSO callback")
+ server.handle_request()
return result📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def wait_for_callback() -> dict[str, str | None]: | |
| result: dict[str, str | None] = {} | |
| class Handler(http.server.BaseHTTPRequestHandler): | |
| def do_GET(self) -> None: | |
| parsed = urllib.parse.urlparse(self.path) | |
| if parsed.path != CALLBACK_PATH: | |
| self.send_response(404) | |
| self.end_headers() | |
| return | |
| qs = urllib.parse.parse_qs(parsed.query) | |
| result["code"] = qs.get("code", [None])[0] | |
| result["state"] = qs.get("state", [None])[0] | |
| result["error"] = qs.get("error", [None])[0] | |
| self.send_response(200) | |
| self.send_header("Content-Type", "text/html") | |
| self.end_headers() | |
| self.wfile.write(b"<html><body>Login captured, you can close this tab.</body></html>") | |
| def log_message(self, *_args: object) -> None: | |
| pass | |
| server = http.server.HTTPServer(("127.0.0.1", CALLBACK_PORT), Handler) | |
| server.timeout = 180 | |
| while "code" not in result and "error" not in result: | |
| server.handle_request() | |
| return result | |
| import time | |
| def wait_for_callback() -> dict[str, str | None]: | |
| result: dict[str, str | None] = {} | |
| class Handler(http.server.BaseHTTPRequestHandler): | |
| def do_GET(self) -> None: | |
| parsed = urllib.parse.urlparse(self.path) | |
| if parsed.path != CALLBACK_PATH: | |
| self.send_response(404) | |
| self.end_headers() | |
| return | |
| qs = urllib.parse.parse_qs(parsed.query) | |
| result["code"] = qs.get("code", [None])[0] | |
| result["state"] = qs.get("state", [None])[0] | |
| result["error"] = qs.get("error", [None])[0] | |
| self.send_response(200) | |
| self.send_header("Content-Type", "text/html") | |
| self.end_headers() | |
| self.wfile.write(b"<html><body>Login captured, you can close this tab.</body></html>") | |
| def log_message(self, *_args: object) -> None: | |
| pass | |
| server = http.server.HTTPServer(("127.0.0.1", CALLBACK_PORT), Handler) | |
| server.timeout = 180 | |
| deadline = time.monotonic() + 180 | |
| while "code" not in result and "error" not in result: | |
| if time.monotonic() > deadline: | |
| raise SystemExit("Timed out waiting for SSO callback") | |
| server.handle_request() | |
| return result |
🤖 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 @.cursor/skills/kartograph/scripts/argocd_pkce_login.py around lines 47 - 73,
Update wait_for_callback so the 180-second timeout is an actual overall deadline
rather than a per-request timeout. Track a monotonic deadline before the request
loop, recompute the remaining time before each server.handle_request() call, and
exit or raise the established timeout error when no callback arrives by the
deadline; preserve immediate completion when code or error is received.
Comment on lines
+103
to
+116
| token_params = { | ||
| "grant_type": "authorization_code", | ||
| "code": result["code"], | ||
| "redirect_uri": redirect_uri, | ||
| "client_id": CLIENT_ID, | ||
| "code_verifier": verifier, | ||
| } | ||
| req = urllib.request.Request( | ||
| f"https://{args.server}/api/dex/token", | ||
| data=urllib.parse.urlencode(token_params).encode(), | ||
| method="POST", | ||
| ) | ||
| with urllib.request.urlopen(req) as resp: | ||
| tokens = json.load(resp) |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- file excerpt ---'
cat -n .cursor/skills/kartograph/scripts/argocd_pkce_login.py | sed -n '85,130p'
echo
echo '--- search for timeout usage in this script/repo ---'
rg -n "urlopen\\(|timeout=|Content-Type|application/x-www-form-urlencoded|ParseForm|dex/token" .cursor/skills/kartograph/scripts/argocd_pkce_login.py .
echo
echo '--- python urllib behavior probe ---'
python3 - <<'PY'
import urllib.request, urllib.parse
token_params = {"grant_type":"authorization_code","code":"abc","redirect_uri":"https://example","client_id":"cid","code_verifier":"ver"}
req = urllib.request.Request(
"https://example.invalid/api/dex/token",
data=urllib.parse.urlencode(token_params).encode(),
method="POST",
)
print("headers:", dict(req.header_items()))
print("has_content_type_header:", req.get_header("Content-type"))
print("method:", req.method)
print("data_type:", type(req.data).__name__, "len:", len(req.data))
PYRepository: openshift-hyperfleet/kartograph
Length of output: 22260
🏁 Script executed (no clone):
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import inspect
import urllib.request
import urllib.parse
import urllib
print("python:", urllib.__file__)
print("Request has data doc:", urllib.request.Request.__doc__.splitlines()[0])
src = inspect.getsource(urllib.request.Request)
for needle in ["content-type", "Content-type", "application/x-www-form-urlencoded", "has_header", "add_header"]:
if needle.lower() in src.lower():
print(f"\n--- matches for {needle} ---")
for i, line in enumerate(src.splitlines(), 1):
if needle.lower() in line.lower():
print(f"{i}: {line}")
PY
Length of output: 309
🏁 Script executed (no clone):
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import inspect, urllib.request
print(inspect.getsource(urllib.request.Request))
PY
Length of output: 3556
🏁 Script executed (no clone):
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import inspect
import urllib.request
for name in ["AbstractHTTPHandler", "HTTPHandler", "HTTPSHandler", "AbstractHTTPHandler.do_request_", "build_opener"]:
obj = getattr(urllib.request, name.split(".")[0], None)
if obj is None:
continue
print("---- AbstractHTTPHandler.do_request_ ----")
print(inspect.getsource(urllib.request.AbstractHTTPHandler.do_request_))
print("---- AbstractHTTPHandler.do_open ----")
print(inspect.getsource(urllib.request.AbstractHTTPHandler.do_open))
PY
Length of output: 4439
🏁 Script executed (no clone):
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import inspect
import urllib.request
print("---- AbstractHTTPHandler.do_request_ ----")
print(inspect.getsource(urllib.request.AbstractHTTPHandler.do_request_))
print("---- AbstractHTTPHandler.do_open ----")
print(inspect.getsource(urllib.request.AbstractHTTPHandler.do_open))
PY
Length of output: 4439
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import inspect
import urllib.request
print("---- urlopen ----")
print(inspect.getsource(urllib.request.urlopen))
print("---- OpenerDirector.open ----")
print(inspect.getsource(urllib.request.OpenerDirector.open))
PYRepository: openshift-hyperfleet/kartograph
Length of output: 4792
Add a finite timeout to the Dex token exchange. urlopen(req) can hang the login flow indefinitely if Dex stalls (CWE-400).
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 114-114: Request-controlled URL passed to urlopen; validate against an allowlist to prevent SSRF.
Context: urllib.request.urlopen(req)
Note: [CWE-918] Server-Side Request Forgery (SSRF).
(urlopen-unsanitized-data)
🤖 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 @.cursor/skills/kartograph/scripts/argocd_pkce_login.py around lines 103 -
116, Add a finite timeout to the urllib.request.urlopen call in the Dex token
exchange so stalled Dex responses cannot block the login flow indefinitely. Keep
the existing request construction and JSON token parsing unchanged.
Comment on lines
+118
to
+121
| id_token = tokens["id_token"] | ||
| with open(args.out, "w", encoding="utf-8") as f: | ||
| f.write(id_token) | ||
| print(f"id_token written to {args.out} ({len(id_token)} chars)") |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Bearer credential written to disk with default file permissions (CWE-276 / CWE-377).
id_token is a live ArgoCD auth token; open(args.out, "w") creates the file with the process umask (commonly world/group-readable) at a predictable, hardcoded default path (/tmp/argocd_token.txt, flagged by static analysis as CWE-377). Any other local user/process with read access to /tmp can lift the token while it's valid.
🔒️ Proposed fix: restrict permissions at creation
+import os
+
- with open(args.out, "w", encoding="utf-8") as f:
- f.write(id_token)
+ fd = os.open(args.out, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600)
+ with os.fdopen(fd, "w", encoding="utf-8") as f:
+ f.write(id_token)Static hints on this line (SSRF for urlopen, path traversal for open) are false positives here: args.server/args.out are operator-supplied CLI flags on a local manual tool, not remote/attacker-controlled input.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| id_token = tokens["id_token"] | |
| with open(args.out, "w", encoding="utf-8") as f: | |
| f.write(id_token) | |
| print(f"id_token written to {args.out} ({len(id_token)} chars)") | |
| import os | |
| id_token = tokens["id_token"] | |
| fd = os.open(args.out, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600) | |
| with os.fdopen(fd, "w", encoding="utf-8") as f: | |
| f.write(id_token) | |
| print(f"id_token written to {args.out} ({len(id_token)} chars)") |
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 118-118: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(args.out, "w", encoding="utf-8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
🤖 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 @.cursor/skills/kartograph/scripts/argocd_pkce_login.py around lines 118 -
121, Update the token-writing flow around id_token and args.out to create the
output file with owner-only permissions (0600) at creation time, rather than
relying on the process umask. Preserve writing the token and reporting its
length, and ensure existing files are not left with broader permissions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
kartographCursor skill (.cursor/skills/kartograph/SKILL.md+argocd_pkce_login.py), previously untracked, covering the ArgoCD SSO login workaround and common REST API endpoints/gotchas for debugging stage.agents.x-k8s.io) as an unowned cluster-side dependency: how to check its live presence via ArgoCD's cluster API-discovery cache, why theopenshell-rbac.yamlRole syncing "Healthy" is not evidence the CRD exists, and that no GitOps repo we have access to (hp-fleet-gitops,hybrid-platforms-gitops/infrastructure,ambient-code-gitops) installs or owns it — so it's a platform/SRE ask, not something to chase in this repo.No application code changes; docs/tooling only.
Test plan
Made with Cursor