Skip to content

fix(security): enigma review findings CTO-4878/4881/4882/4884/4885/4886/4889 - #358

Merged
vedharish merged 4 commits into
mainfrom
fix/cto-enigma-security-batch-3
Nov 12, 2026
Merged

vedharish merged 4 commits into
mainfrom
fix/cto-enigma-security-batch-3

Conversation

@Vishalan

@Vishalan Vishalan commented Nov 12, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Fixes the open enigma security-review findings.

Ticket Finding Change
CTO-4889 F-041 duplicate default-group membership race Membership check is now a DB query (not str(QuerySet)), done with the create under transaction.atomic() and a select_for_update lock on the user row. Access grant runs after the lock is released.
CTO-4881 F-033 mark_revoked always 403 Returns 200 on success, 400 for missing username, 500 on exception.
CTO-4882 F-034 password history Slice is now [:3]. password_changed compared a freshly salted hash, so it never matched and stored a duplicate each time; it now compares against the latest stored hash.
CTO-4884 F-036 CID from random.random() uuid.uuid4().hex.
CTO-4885 F-037 CI installs module requirements with --ignore-installed Flag dropped in CI and docker-entrypoint.sh; module requirements are installed with -c requirements.txt, so core security pins win and a module cannot downgrade them. Core pin six 1.15.0 -> 1.17.0 (1.15 has no six.moves on Python 3.12+; --ignore-installed had been hiding this by pulling a newer six).
CTO-4886 F-038 pre-commit autoupdate in CI Step removed; both hook revs pinned to commit SHAs.
CTO-4878 F-030 dev tools in image vim and curl removed (see limits).
CTO-4860 F-012 secrets in image No change: .dockerignore already on main (3d85dfc).

Limits (not fully closed by this PR)

  • CTO-4882: Django applies the step of a queryset slice to the already limited list, so [:3:-1] still checked the last 3 passwords. The "check bypassed" claim does not reproduce; the real bug was the duplicate rows. Test test_recent_password_is_rejected_even_with_short_history passes before and after.
  • CTO-4878: git, openssh-client, gcc/build-essential stay. docker-entrypoint.sh clones the access modules and pip-installs their requirements at container start. Removing them needs those steps moved to build time (follow-up).
  • CTO-4885: Module requirements are still not hash-pinned and clone SHAs are not pinned (F-019).
  • CTO-4889: No DB unique constraint added: MembershipV2 legitimately holds several rows per (group, user) across statuses, and existing duplicates would break the migration.
  • CTO-4860: Images already published to DockerHub before the fix still contain the secrets; those credentials need rotating and old tags removing (ops action).

Testing

  • New: Access/tests/test_security_batch3.py, rewritten bootprocess/tests/test_views_helper.py (real DB instead of model monkeypatching). 6 of the new tests fail on the old code.
  • Local venv: 151 passed, 8 skipped. pylama (CI scope) clean.
  • Docker make test: 151 passed, 8 skipped; make lint passed. Image has no vim or curl.

🤖 Generated with Claude Code

…y chain (CTO-4878/4881/4882/4884/4885/4886/4889)

- F-041 get_dashboard_data: DB membership check + row lock in one transaction
- F-033 mark_revoked: return real status (200/400/500) instead of always 403
- F-034 RepeatedValidator: [:3] slice; password_changed stops storing duplicates
- F-036 CID_GENERATOR: uuid4 instead of random.random()
- F-037 CI: drop --ignore-installed for access-module requirements
- F-038 CI: remove pre-commit autoupdate, pin hook revs to commit SHAs
- F-030 Dockerfile: drop vim and curl from the image

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Nov 12, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

  • Source mode: Auto (fallback from Default branch)
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Central YAML (base), Organization UI (inherited), Workspace UI (inherited); Enforced overrides: Organization
  • Review profile: ASSERTIVE
  • Plan: Enterprise
  • Run ID: b006c9d3-5f30-4d69-9926-70c063e471ff

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

…3.12+ (CTO-4885 needs a proper fix)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@Vishalan Vishalan changed the title fix(security): enigma review findings CTO-4878/4881/4882/4884/4885/4886/4889 fix(security): enigma review findings CTO-4878/4881/4882/4884/4886/4889 Nov 12, 2026
Vishalan and others added 2 commits November 12, 2026 15:56
… --ignore-installed (F-037)

Module requirements can no longer downgrade or override security-pinned core packages.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@Vishalan Vishalan changed the title fix(security): enigma review findings CTO-4878/4881/4882/4884/4886/4889 fix(security): enigma review findings CTO-4878/4881/4882/4884/4885/4886/4889 Nov 12, 2026
@vedharish
vedharish merged commit 728a145 into main Nov 12, 2026
9 checks passed
@vedharish
vedharish deleted the fix/cto-enigma-security-batch-3 branch November 12, 2026 11:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants