fix(views): correct container reuse logic, fix flag validation, subprocess errors, port range, cleanup#443
Open
suradadhanush wants to merge 2 commits into
Conversation
…errors, port range, cleanup - Reorder post() to attempt docker start before any docker run — prevents orphaned containers - Remove broken docker ps short ID vs full ID comparison — use direct restart returncode instead - Fix flag validation: replace request.POST.get with json.loads(request.body) for PUT requests - Add stderr=subprocess.PIPE and returncode checks to all subprocess calls - Start port scan from 8001 to avoid collision with app on 8000 - Remove unused import csrf_exempt (Flake8 F401) - Fix typo: 'challange' -> 'challenge' in put() parameter
- ChallengeModelTest: validates flag hashing and port range enforcement - FlagValidationTest: 5 cases covering correct/wrong/empty/unauth/invalid-JSON - ContainerReuseTest: mocks subprocess to verify docker start before docker run docs: add docs/API_DOCUMENTATION.md for challenge system endpoints feat(settings): add security headers (XSS, HSTS, X-Frame-Options, NOSNIFF)
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.
Changes
Container Reuse (resolves TODO in post())
The previous implementation called
docker rununconditionally before checking if a container could be reused — causing orphaned containers on every reuse-eligible POST. Additionally, the reuse check compared a 12-char short ID fromdocker psagainst the full 64-char container ID stored in the database, so reuse never triggered even when the container was healthy.Fix: Attempt
docker startdirectly first.docker startreturns non-zero if the container doesn't exist — this is used as the fallback signal to create a new one. Nodocker pscheck needed.Flag Validation (resolves TODO in put())
Django does not parse PUT request bodies into
request.POST.request.POST.get('flag')always returnsNoneon a PUT request, causing the endpoint to always return 400.Fix: Parse the request body directly using
json.loads(request.body). SHA-256 hash comparison against the storedhashed_prefixed flag is preserved correctly.Subprocess error handling
Added
stderr=subprocess.PIPEandreturncode != 0checks to allsubprocess.Popencalls — previously silent failures returned 200 with a garbagecontainer_id.Other fixes
8001instead of8000— app runs on8000, scanning from8000wastes a check every timecsrf_exempt(Flake8 F401)challange→challengeinput()method parameter