fix: resolve CodeQL code-quality findings (with tests) - #23
Merged
Conversation
Address code scanning alerts from the security-and-quality suite: - Remove unused imports (json, check_ssh_agent, success, error in setup.py; re, success, format_instance_id in cli.py; Path in core.py and _1password.py) - Remove redundant nested 'import re' in setup.py (already imported at module top) - Fix unreachable statement in _create_1password_key: the SSH-agent reminder and its return were dead code after both branches returned; restructured so the reminder is shown before returning True - Replace two bare 'except:' blocks with 'except Exception:' so KeyboardInterrupt/SystemExit propagate Add pytest suite (tests/test_setup.py) covering the behavioural changes: - _create_1password_key success path now saves the key AND shows the reminder (regression test fails against the old unreachable code) - setup_aws_profile handles a missing profile but lets KeyboardInterrupt propagate (verifies the narrowed except) - validate_instance_id valid/invalid cases Add .github/workflows/test.yml running pytest on Python 3.10-3.13 for pushes and PRs, gating merges to main/develop. The high-severity 'clear-text logging of sensitive data' alert is a false positive: the flagged strings contain the product name '1Password', not an actual password. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| def _install_stubs(self, monkeypatch, setup, *, create_success=True, | ||
| save_success=True): | ||
| import cloudx_proxy.setup as setup_mod |
| """The bare except was narrowed to Exception (issue #2 in CodeQL).""" | ||
|
|
||
| def test_missing_profile_is_handled(self, monkeypatch, setup): | ||
| import cloudx_proxy.setup as setup_mod |
| assert isinstance(result, bool) | ||
|
|
||
| def test_keyboard_interrupt_propagates(self, monkeypatch, setup): | ||
| import cloudx_proxy.setup as setup_mod |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses CodeQL “security-and-quality” findings in the CloudX setup flow (notably exception handling and a previously unreachable 1Password reminder), and adds a pytest-backed regression suite plus CI to ensure the fixes stay covered across Python 3.10–3.13.
Changes:
- Fixes CodeQL-reported quality issues in
CloudXSetup(unreachable reminder code; replace bareexcept:withexcept Exception:; remove redundant/unused imports). - Adds a focused pytest suite covering the corrected 1Password success path, narrowed exception handling behavior, and instance-id validation.
- Introduces a GitHub Actions workflow to run pytest on pushes/PRs to
main/develop, and wires pytest into the dev dependency group.
Reviewed changes
Copilot reviewed 7 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
cloudx_proxy/setup.py |
Fixes unreachable reminder logic, narrows exception handling, and removes redundant/unused imports per CodeQL. |
cloudx_proxy/cli.py |
Removes unused imports tied to CodeQL notes. |
cloudx_proxy/core.py |
Removes unused Path import. |
cloudx_proxy/_1password.py |
Removes unused Path import. |
tests/test_setup.py |
Adds regression tests for the touched behaviors (1Password key creation reminder, exception handling, instance-id validation). |
tests/__init__.py |
Establishes tests as a package (empty init). |
pyproject.toml |
Adds pytest to the dev group and configures pytest discovery/options. |
uv.lock |
Locks pytest and its transitive dependencies. |
.github/workflows/test.yml |
Adds CI job to run pytest on Python 3.10–3.13 for pushes/PRs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+27
to
+33
| - name: Checkout | ||
| uses: actions/checkout@v7 | ||
|
|
||
| - name: Install uv | ||
| uses: astral-sh/setup-uv@v7 | ||
| with: | ||
| python-version: ${{ matrix.python-version }} |
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
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
Resolves the code-quality alerts surfaced by the new CodeQL scan, backed by a test suite since users rely on this tool. Nothing goes to
mainuntil CI (pytest on Python 3.10–3.13) is green and this PR is reviewed.CodeQL findings addressed
_create_1password_keyreturn Trueexcept:handlesBaseExceptionexcept Exception:soKeyboardInterrupt/SystemExitpropagateimport rejson,check_ssh_agent,success,error,re,format_instance_id,Path)Alert #1 (high, "clear-text logging of sensitive data") is a false positive — the flagged strings contain the product name 1Password, not a real password. Recommend dismissing it as "false positive" in the Security tab.
Tests added (
tests/test_setup.py)_create_1password_key: success path saves the key and shows the reminder. This test fails against the old unreachable-code version (verified locally) and passes against the fix — it genuinely guards the behaviour change.setup_aws_profile: a missing profile is still handled, butKeyboardInterruptnow propagates (verifies the narrowedexcept).validate_instance_id: valid/invalid regression cases.16 tests, all passing locally.
CI
New
.github/workflows/test.ymlrunspyteston Python 3.10–3.13 for pushes and PRs tomain/develop.Notes
pytestadded to thedevdependency group.🤖 Generated with Claude Code