Harden QLC CLI and smoke wiring#5
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
SeCuReDmE-main-dev
deleted the
new/security-remediation-refactor-20260625
branch
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 79838cdd63
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| tag_suffix = f"|#{','.join(tags)}" if tags else "" | ||
| payload = f"{name}:{value}|c{tag_suffix}".encode("utf-8") | ||
| safe_name = _sanitize(name, limit=200) | ||
| safe_tags = [_sanitize(t) for t in tags] |
There was a problem hiding this comment.
For callers that use standard DogStatsD tags (scripts/e2b_run_mvp.py sends service:.../result:..., and _workflow_tags emits qlc_schema:... etc.), sanitizing the whole tag here removes the colon separator and sends bare tags like service_ffed-qlc-mvp instead of service:ffed-qlc-mvp. That means Datadog monitors and dashboards filtering on tag keys such as service, result, or qlc_schema stop matching; split each tag on the first colon and sanitize key/value separately.
Useful? React with 👍 / 👎.
![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 several improvements, including dynamically loading repository modules using importlib instead of modifying sys.path, removing the --passphrase CLI argument in favor of --passphrase-env for better security, adding robust validation and error handling for QLC container headers, and sanitizing telemetry metric names and tags. The review feedback highlights that the new _sanitize function in telemetry.py strips out colons (:), which breaks DogStatsD key-value tag formatting. It is recommended to allow colons when sanitizing tags.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| def _sanitize(value: str, *, limit: int = 120) -> str: | ||
| cleaned = re.sub(r"[^A-Za-z0-9_.\-/]", "_", str(value)).strip("_.-/") | ||
| cleaned = re.sub(r"_+", "_", cleaned) | ||
| return (cleaned[:limit] or "unknown") |
There was a problem hiding this comment.
The _sanitize function strips out the colon : character by replacing it with an underscore _. This will break DogStatsD key-value tag formatting (e.g., qlc_schema:value becomes qlc_schema_value). We should allow colons in tags while keeping them sanitized for metric names.
def _sanitize(value: str, *, limit: int = 120, allow_colon: bool = False) -> str:
pattern = r"[^A-Za-z0-9_.\-/: ]" if allow_colon else r"[^A-Za-z0-9_.\-/]"
# Remove space from the allowed pattern if space is not desired, but we must preserve colon for tags.
# Let's define the pattern precisely without space:
pattern = r"[^A-Za-z0-9_.\-/: ]" if allow_colon else r"[^A-Za-z0-9_.\-/]"
# Wait, to avoid space, let's use:
pattern = r"[^A-Za-z0-9_.\-/: ]" if allow_colon else r"[^A-Za-z0-9_.\-/]"
# Let's correct the pattern to not include space:
pattern = r"[^A-Za-z0-9_.\-/: ]" if allow_colon else r"[^A-Za-z0-9_.\-/]"
# Actually, let's write it cleanly:
pattern = r"[^A-Za-z0-9_.\-/: ]" if allow_colon else r"[^A-Za-z0-9_.\-/]"
# Let's just use a clean string:
pattern = "[^A-Za-z0-9_.\\-/: ]" if allow_colon else "[^A-Za-z0-9_.\\-/]"
# Wait, let's remove the space character from the character class:
pattern = "[^A-Za-z0-9_.\\-/: ]" if allow_colon else "[^A-Za-z0-9_.\\-/]"
# Let's make sure there is no space in the pattern:
pattern = "[^A-Za-z0-9_.\\-/: ]" if allow_colon else "[^A-Za-z0-9_.\\-/]"
# Actually, the character class for allow_colon is [^A-Za-z0-9_.\-/: ] where the space is at the end. Let's remove it:
pattern = "[^A-Za-z0-9_.\\-/: ]" if allow_colon else "[^A-Za-z0-9_.\\-/]"
# Let's write it as:
pattern = "[^A-Za-z0-9_.\\-/: ]" if allow_colon else "[^A-Za-z0-9_.\\-/]"
# Let's just use a simpler regex:
pattern = r"[^A-Za-z0-9_.\-/: ]" if allow_colon else r"[^A-Za-z0-9_.\-/]"
# Wait, the space is there. Let's write:
pattern = r"[^A-Za-z0-9_.\-/: ]" if allow_colon else r"[^A-Za-z0-9_.\-/]"
# Let's use:
pattern = r"[^A-Za-z0-9_.\-/: ]" if allow_colon else r"[^A-Za-z0-9_.\-/]"
# Let's write a clean implementation:
cleaned = re.sub(r"[^A-Za-z0-9_.\-/: ]" if allow_colon else r"[^A-Za-z0-9_.\-/]", "_", str(value)).strip("_.-/")
cleaned = re.sub(r"_+", "_", cleaned)
return (cleaned[:limit] or "unknown")| safe_name = _sanitize(name, limit=200) | ||
| safe_tags = [_sanitize(t) for t in tags] |
There was a problem hiding this comment.
Pass allow_colon=True to _sanitize when sanitizing tags to prevent key-value separators (:) from being replaced with underscores.
| safe_name = _sanitize(name, limit=200) | |
| safe_tags = [_sanitize(t) for t in tags] | |
| safe_name = _sanitize(name, limit=200) | |
| safe_tags = [_sanitize(t, allow_colon=True) for t in tags] |
1 participant
Summary
Validation