fix: resolve 1.0 audit bugs (closes #104, #105, #106, #107, #108) #109

Merged
sneak merged 6 commits from fix/1.0-audit-bugs into main 2026-02-20 13:47:12 +01:00
Collaborator

Fixes 5 critical/high bugs for the 1.0 release:

  • #104 (CRITICAL): HandleEnvVarDelete used wrong route param envID instead of varID
  • #105 (HIGH): API deploy handler used request context instead of detached context
  • #106 (HIGH): API delete endpoint did not clean up Docker container before DB deletion
  • #107 (HIGH): HandleVolumeAdd was missing path validation (unlike HandleVolumeEdit)
  • #108 (HIGH): SetupRequired middleware blocked /health, /s/*, and /api/* before setup

Also fixes pre-existing lint issues (cyclop, funlen, testpackage).

make check passes with zero issues.

Fixes 5 critical/high bugs for the 1.0 release: - **#104 (CRITICAL):** `HandleEnvVarDelete` used wrong route param `envID` instead of `varID` - **#105 (HIGH):** API deploy handler used request context instead of detached context - **#106 (HIGH):** API delete endpoint did not clean up Docker container before DB deletion - **#107 (HIGH):** `HandleVolumeAdd` was missing path validation (unlike `HandleVolumeEdit`) - **#108 (HIGH):** `SetupRequired` middleware blocked `/health`, `/s/*`, and `/api/*` before setup Also fixes pre-existing lint issues (cyclop, funlen, testpackage). `make check` passes with zero issues.
clawbot self-assigned this 2026-02-20 12:36:00 +01:00
clawbot added 6 commits 2026-02-20 12:36:00 +01:00
Author
Collaborator

Code Review: PR #109 (fix/1.0-audit-bugs)

Reviewer: clawbot | Result: LGTM, merge ready

Fix-by-fix review

  • #104 env var delete param: envID to varID in chi.URLParam(). Matches route {varID}. Test: TestHandleEnvVarDeleteUsesCorrectRouteParam. CORRECT.
  • #105 detached deploy context: context.WithoutCancel(). HTTP disconnect will not kill deploy. Test: TestAPITriggerDeployUsesDetachedContext. CORRECT.
  • #106 container cleanup on delete: cleanupContainer() called before DeleteApp(). Docker cleanup before DB delete. CORRECT.
  • #107 volume path validation: validateVolumePaths() added to HandleVolumeAdd. Same validation as edit path. Test: TestHandleVolumeAddValidatesPaths (4 cases). CORRECT.
  • #108 setup middleware exemptions: /health, /s/, /api/ exempted from setup redirect. Test: TestSetupRequiredExemptsHealthAndStaticAndAPI. CORRECT.

Additional changes (all good)

  • apiCreateRequest + validateCreateRequest extracted from handler
  • repo_url_validation: switch to map for allowed schemes
  • repo_url_validation_test moved to _test package with export_test.go
  • allowedRepoSchemes //nolint:gochecknoglobals is legitimate (constant map)

Safety checks

  • No .golangci.yml or linter config modified
  • No illegitimate //nolint annotations added
  • make check passes clean (all tests pass, build succeeds)
  • No regressions introduced

Ship it.

## Code Review: PR #109 (fix/1.0-audit-bugs) **Reviewer:** clawbot | **Result:** LGTM, merge ready ### Fix-by-fix review - **#104** env var delete param: `envID` to `varID` in chi.URLParam(). Matches route {varID}. Test: TestHandleEnvVarDeleteUsesCorrectRouteParam. CORRECT. - **#105** detached deploy context: context.WithoutCancel(). HTTP disconnect will not kill deploy. Test: TestAPITriggerDeployUsesDetachedContext. CORRECT. - **#106** container cleanup on delete: cleanupContainer() called before DeleteApp(). Docker cleanup before DB delete. CORRECT. - **#107** volume path validation: validateVolumePaths() added to HandleVolumeAdd. Same validation as edit path. Test: TestHandleVolumeAddValidatesPaths (4 cases). CORRECT. - **#108** setup middleware exemptions: /health, /s/*, /api/* exempted from setup redirect. Test: TestSetupRequiredExemptsHealthAndStaticAndAPI. CORRECT. ### Additional changes (all good) - apiCreateRequest + validateCreateRequest extracted from handler - repo_url_validation: switch to map for allowed schemes - repo_url_validation_test moved to _test package with export_test.go - allowedRepoSchemes //nolint:gochecknoglobals is legitimate (constant map) ### Safety checks - No .golangci.yml or linter config modified - No illegitimate //nolint annotations added - make check passes clean (all tests pass, build succeeds) - No regressions introduced Ship it.
clawbot added the
merge-ready
label 2026-02-20 12:38:49 +01:00
clawbot removed their assignment 2026-02-20 12:38:50 +01:00
sneak was assigned by clawbot 2026-02-20 12:38:50 +01:00
sneak merged commit 4217e62f27 into main 2026-02-20 13:47:12 +01:00
Sign in to join this conversation.
No description provided.