Skip to content

Code Review: Security + Documentation + CI/CD #208

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mikejmorgan-ai merged 1 commit into from
Nov 28, 2025
Merged

Code Review: Security + Documentation + CI/CD #208

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 72 additions & 14 deletions .github/workflows/automation.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,24 +9,82 @@ on:
jobs:
test:
runs-on: ubuntu-latest

strategy:
matrix:
python-version: ['3.10', '3.11', '3.12']

steps:
- uses: actions/checkout@v3
- name: Set up Python
uses: actions/setup-python@v4
- uses: actions/checkout@v4

- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v5
with:
python-version: '3.11'
python-version: ${{ matrix.python-version }}

- name: Install dependencies
run: |
python -m pip install --upgrade pip
if [ -f requirements.txt ]; then pip install -r requirements.txt; fi

pip install -r requirements.txt
pip install pytest pytest-cov pytest-mock

- name: Run tests
run: |
if [ -d tests ]; then
python -m pytest tests/ || echo "Tests not yet implemented"
else
echo "No tests directory found"
fi
python -m pytest test/ -v --cov=cortex --cov-report=xml --cov-report=term-missing

- name: Upload coverage to Codecov
uses: codecov/codecov-action@v4
if: matrix.python-version == '3.11'
with:
file: ./coverage.xml
fail_ci_if_error: false

lint:
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v4

- name: Set up Python
uses: actions/setup-python@v5
with:
python-version: '3.11'

- name: Install linting tools
run: |
python -m pip install --upgrade pip
pip install black pylint mypy

- name: Check formatting with black
run: |
black --check cortex/ || echo "::warning::Code formatting issues found. Run 'black cortex/' to fix."

- name: Lint with pylint
run: |
pylint cortex/ --exit-zero --output-format=text | tee pylint-report.txt
score=$(tail -n 2 pylint-report.txt | head -n 1 | grep -oP '\d+\.\d+')
echo "Pylint score: $score"
Comment on lines +52 to +65
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 28, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Lint job doesn't install project dependencies, and mypy is never run.

The lint job installs black pylint mypy but doesn't install the project or its dependencies. This will cause pylint and mypy to report false positives for missing imports. Additionally, mypy is installed but never executed.

     - name: Install linting tools
       run: |
         python -m pip install --upgrade pip
+        pip install -r requirements.txt
         pip install black pylint mypy

     - name: Check formatting with black
       run: |
         black --check cortex/ || echo "::warning::Code formatting issues found. Run 'black cortex/' to fix."

     - name: Lint with pylint
       run: |
         pylint cortex/ --exit-zero --output-format=text | tee pylint-report.txt
         score=$(tail -n 2 pylint-report.txt | head -n 1 | grep -oP '\d+\.\d+')
         echo "Pylint score: $score"
+
+    - name: Type check with mypy
+      run: |
+        mypy cortex/ --ignore-missing-imports || echo "::warning::Type errors found."
📝 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
- name: Install linting tools
run: |
python -m pip install --upgrade pip
pip install black pylint mypy
- name: Check formatting with black
run: |
black --check cortex/ || echo "::warning::Code formatting issues found. Run 'black cortex/' to fix."
- name: Lint with pylint
run: |
pylint cortex/ --exit-zero --output-format=text | tee pylint-report.txt
score=$(tail -n 2 pylint-report.txt | head -n 1 | grep -oP '\d+\.\d+')
echo "Pylint score: $score"
- name: Install linting tools
run: |
python -m pip install --upgrade pip
pip install -r requirements.txt
pip install black pylint mypy
- name: Check formatting with black
run: |
black --check cortex/ || echo "::warning::Code formatting issues found. Run 'black cortex/' to fix."
- name: Lint with pylint
run: |
pylint cortex/ --exit-zero --output-format=text | tee pylint-report.txt
score=$(tail -n 2 pylint-report.txt | head -n 1 | grep -oP '\d+\.\d+')
echo "Pylint score: $score"
- name: Type check with mypy
run: |
mypy cortex/ --ignore-missing-imports || echo "::warning::Type errors found."
🤖 Prompt for AI Agents 
In .github/workflows/automation.yml around lines 52 to 65 the lint job installs
only black/pylint/mypy but does not install the project or its dependencies and
also never runs mypy; update the workflow to install project deps (for example
run pip install -r requirements.txt and/or pip install -e .[dev] or the
project's chosen install command) before running linters, and add a step to
execute mypy (e.g. mypy cortex/ with the desired flags) so both pylint and mypy
see installed packages and run correctly.


security:
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v4

- name: Set up Python
uses: actions/setup-python@v5
with:
python-version: '3.11'

- name: Install security tools
run: |
python -m pip install --upgrade pip
pip install bandit safety

- name: Run Bandit security linter
run: |
bandit -r cortex/ -ll -ii || echo "::warning::Security issues found. Please review."

- name: Check dependencies with safety
run: |
pip install -r requirements.txt
safety check --full-report || echo "::warning::Vulnerable dependencies found."
Comment on lines +83 to +90
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 28, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Security checks don't fail CI, undermining their purpose.

Both bandit and safety use || echo "::warning::" which allows the workflow to pass even when security issues are found. For a PR focused on security hardening, this is counterproductive.

     - name: Run Bandit security linter
       run: |
-        bandit -r cortex/ -ll -ii || echo "::warning::Security issues found. Please review."
+        bandit -r cortex/ -ll -ii

     - name: Check dependencies with safety
       run: |
         pip install -r requirements.txt
-        safety check --full-report || echo "::warning::Vulnerable dependencies found."
+        safety check --full-report

If you need to allow the workflow to complete for visibility while still marking it as failed, use continue-on-error: true on the step instead.

📝 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
- name: Run Bandit security linter
run: |
bandit -r cortex/ -ll -ii || echo "::warning::Security issues found. Please review."
- name: Check dependencies with safety
run: |
pip install -r requirements.txt
safety check --full-report || echo "::warning::Vulnerable dependencies found."
- name: Run Bandit security linter
run: |
bandit -r cortex/ -ll -ii
- name: Check dependencies with safety
run: |
pip install -r requirements.txt
safety check --full-report
🤖 Prompt for AI Agents 
In .github/workflows/automation.yml around lines 83-90, the Bandit and Safety
steps currently swallow failures via "|| echo '::warning::...'", so security
checks never fail CI; remove those trailing "|| echo" fallbacks and let the
commands return non-zero on issues, and if you still want the workflow to finish
for visibility add continue-on-error: true to each respective step (while
keeping the commands plain so they fail when issues are found).

Loading
Loading