feat: CI/CD improvements with coverage, bandit, and safety#18
feat: CI/CD improvements with coverage, bandit, and safety#18sadykovIsmail merged 1 commit intomainfrom
Conversation
…ety audit - Split CI into three parallel jobs: test, lint, security - Test job: runs coverage >= 80% threshold, uploads XML report to Codecov - Lint job: flake8 with max-line-length=100, W503 ignored - Security job: bandit SAST (medium+ severity) + safety dependency audit - Add .coveragerc: omit migrations, manage.py, wsgi/asgi, test files - Update .flake8: max-line-length=100, extend-ignore W503 - Update requirements.dev.txt: add coverage, bandit, safety - Trigger on push to any branch and on PRs targeting main Closes #9
There was a problem hiding this comment.
Pull request overview
This PR improves the project’s GitHub Actions CI by splitting checks into parallel jobs and adding coverage + security tooling to better enforce quality gates for the Django codebase.
Changes:
- Split the previous combined job into separate Test & Coverage, Lint (flake8), and Security Scan (bandit + safety) jobs.
- Add dev/CI dependencies for coverage and security scanning (
coverage,bandit,safety). - Introduce/adjust tool configuration via
.coveragercand.flake8.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
.github/workflows/checks.yml |
New CI workflow structure with parallel jobs, coverage generation/upload, and security scans. |
requirements.dev.txt |
Adds CI/dev tools for coverage and security scanning. |
app/.coveragerc |
Defines coverage source/omit rules and an 80% minimum threshold. |
app/.flake8 |
Updates flake8 settings (line length, excludes, ignore list). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - name: Login to Docker Hub | ||
| uses: docker/login-action@v3 | ||
| with: | ||
| username: ${{ secrets.DOCKERHUB_USER }} | ||
| password: ${{ secrets.DOCKERHUB_TOKEN }} |
There was a problem hiding this comment.
The Docker Hub login step will fail on pull_request events from forks because secrets (DOCKERHUB_USER/DOCKERHUB_TOKEN) are not provided to forked PRs. Consider guarding this step with an if: that checks secrets are present / PR is from same repo, or remove the login entirely to avoid breaking CI for external contributors.
| - name: Run tests with coverage | ||
| run: | | ||
| docker compose run --rm app sh -c " | ||
| coverage run --source=. --omit='*/migrations/*,manage.py,app/settings.py' \ | ||
| manage.py test && | ||
| coverage report --fail-under=80 && | ||
| coverage xml -o /app/coverage.xml | ||
| " |
There was a problem hiding this comment.
coverage run writes its data file (and your coverage xml -o /app/coverage.xml output) into /app, which is a bind mount from the GitHub workspace. Since the container runs as django-user (non-root), it often won’t have write permission to the mounted workspace directory in CI, causing coverage generation to fail. Consider running the coverage commands with a writable COVERAGE_FILE/output dir (e.g., /tmp) and/or running the container with a UID that matches the runner (or as root for this step).
| coverage run --source=. --omit='*/migrations/*,manage.py,app/settings.py' \ | ||
| manage.py test && | ||
| coverage report --fail-under=80 && |
There was a problem hiding this comment.
The workflow passes --source/--omit/--fail-under on the command line even though a .coveragerc was added. Coverage CLI options override config, so the omit list in .coveragerc (e.g., excluding tests) won’t apply, and the workflow and config can easily drift. Prefer relying on .coveragerc (and keep a single source of truth) or update the config to match the CLI and remove the flags here.
| coverage run --source=. --omit='*/migrations/*,manage.py,app/settings.py' \ | |
| manage.py test && | |
| coverage report --fail-under=80 && | |
| coverage run manage.py test && | |
| coverage report && |
| run: | | ||
| docker compose run --rm app sh -c " | ||
| bandit -r . \ | ||
| --exclude ./migrations,./tests \ |
There was a problem hiding this comment.
The Bandit exclude paths don’t match this repo’s layout: there is no top-level ./migrations directory (migrations live under app modules like core/migrations). As written, Bandit will still scan migrations (and likely tests), which can add noise/false positives. Adjust --exclude to cover */migrations/* and */tests/* (or the specific app paths) based on the actual tree under /app.
| --exclude ./migrations,./tests \ | |
| --exclude '*/migrations/*,*/tests/*' \ |
| - name: Copy coverage report from container | ||
| if: always() | ||
| run: | | ||
| docker compose run --rm -v ${{ github.workspace }}:/workspace app sh -c \ | ||
| "cp /app/coverage.xml /workspace/coverage.xml" || true |
There was a problem hiding this comment.
This step spawns an extra docker compose run just to copy a file, which can also re-trigger dependency containers (e.g., Postgres) and is sensitive to the same UID/permissions issue as the coverage generation. Since /app is already a bind mount, the coverage XML should already exist on the host after the previous step (likely under app/coverage.xml); consider copying it directly on the runner (or writing it to the workspace path in the first place) instead of starting a new container.
| - name: Copy coverage report from container | |
| if: always() | |
| run: | | |
| docker compose run --rm -v ${{ github.workspace }}:/workspace app sh -c \ | |
| "cp /app/coverage.xml /workspace/coverage.xml" || true |
Summary
Splits the single CI job into three focused parallel jobs and adds security scanning + coverage reporting.
CI Jobs
1. Test & Coverage
coveragecoverage.xmlto Codecov2. Lint (flake8)
max-line-length = 100__pycache__,manage.py,settings.pyW503(conflicts with Black style)3. Security Scan
Files Changed
.github/workflows/checks.ymlrequirements.dev.txtcoverage,bandit,safetyapp/.coveragercapp/.flake8Test plan
Closes #9