Update README.md#7
Merged
Merged
Conversation
vanajmoorthy
added a commit
that referenced
this pull request
May 15, 2026
Addresses all 4 BLOCKER + several MEDIUM findings from the post-Phase-1 security review. The original PR's default config didn't actually close the anonymous-DNA-hijack vulnerability — this commit closes it for real. Blocker fixes ============= #1 — `get_task_result_view` no longer writes DNA into the session on ownership mismatch (regardless of `ENFORCE_TASK_OWNERSHIP`). The old warn-and-allow path wrote `session["dna_data"]` even when the caller wasn't the owner, which combined with signup_view's `if "dna_data" in request.session:` branch to re-open the hijack. Now: cache miss or mismatch → 403 + log, no session writes. `ENFORCE_TASK_OWNERSHIP` is retained for deploy-doc compat but no longer affects the response (documented in settings.py). #2 — `claim_anonymous_dna_task(session_key)` is now a REQUIRED positional arg (was `session_key: str = None`). Previously, omitting it silently bypassed the ownership check. Direct broker publishers can no longer skip the check by leaving the arg off. #3 — Both the view and the task now fail closed on cache miss. The 1hr TTL on `task_owner_<id>` used to be a security hole: after expiry the check silently passed because `owner_session_key is None`. Now treated as "task unowned, reject" — the user re-uploads if their TTL ran out (the cached DNA at `dna_result_<id>` has the same TTL anyway). #4 — Session keys are session bearer credentials. They are no longer logged raw. Both views and tasks now hash with sha256[:12] before emitting any log line. Forensic value preserved; credential leak eliminated. Medium fixes ============ #6 — `signup_view` now ALSO checks `task_owner_<task_id>` against the caller's session_key before accepting `task_id_to_claim`. Layered with the existing `session["anonymous_task_id"]` consistency check for defense in depth. #7 — DEBUG production assert widened. Was `DJANGO_ENV == "production"` exact match (so `DJANGO_ENV=staging` silently allowed DEBUG=True). Now uses a whitelist of permitted DEBUG envs (`development`, `test`, `ci`); anything else with DEBUG=True raises ImproperlyConfigured. Also adds a `_env_bool` helper that fails loud on unrecognized values — typoed `ENFORCE_TASK_OWNERSHIP=true` (lowercase) no longer silently parses as False. #12 — On any rejection in `claim_anonymous_dna_task` (missing key, cache miss, or mismatch), `pending_dna_task_id` is cleared so the rightful user's dashboard doesn't poll forever. Polish ====== #10 — Removed hardcoded absolute username path from RALPH_PROMPT.md. #11 — Added `scripts/ralph/`, `scripts/regression/`, `tasks/`, `progress.txt`, `AGENTS.md`, `.claude/` to .dockerignore so the internal triage scaffolding doesn't ship in production images. Also added `.claude/worktrees/` and `scripts/ralph/.last-branch` to .gitignore. Tests ===== Updated `test_task_result_warns_but_allows_when_not_enforced` to `test_task_result_rejects_foreign_session_even_when_not_enforced` — the original test was happy-path-shaped and would have passed against the buggy fix. New assertions verify the attacker's session has NO leaked DNA data after the 403 response (finding #9). New negative tests: - `test_claim_anonymous_dna_task_fails_closed_on_cache_miss` - `test_claim_anonymous_dna_task_fails_closed_on_missing_session_key` Updated positive `test_claim_anonymous_dna_task` to pass a session_key arg (was 2-arg, would have failed after #2's required-arg change). Full suite: 359 tests, all green.
5 tasks
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
1 participant
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.
No description provided.