feat(security): seed default SecurityConfig row on application startup#852
Merged
Wikid82 merged 11 commits intodevelopmentfrom Mar 17, 2026
Merged
feat(security): seed default SecurityConfig row on application startup#852Wikid82 merged 11 commits intodevelopmentfrom
Wikid82 merged 11 commits intodevelopmentfrom
Conversation
On a fresh install the security_configs table is auto-migrated but contains no rows. Any code path reading SecurityConfig by name received an empty Go struct with zero values, producing an all-disabled UI state that offered no guidance to the user and made the security status endpoint appear broken. Adds a SeedDefaultSecurityConfig function that uses FirstOrCreate to guarantee a default row exists with safe, disabled-by-default values on every startup. The call is idempotent — existing rows are never modified, so upgrades are unaffected. If the seed fails the application logs a warning and continues rather than crashing. Zero-valued rate-limit fields are intentional and safe: the Cerberus rate-limit middleware applies hardcoded fallback thresholds when the stored values are zero, so enabling rate limiting without configuring thresholds results in sensible defaults rather than a divide-by-zero or traffic block. Adds three unit tests covering the empty-database, idempotent, and do-not-overwrite-existing paths.
Contributor
There was a problem hiding this comment.
Pull request overview
Seeds a default SecurityConfig row on application startup to fix a fresh-install bug where the security status endpoint returned confusing all-disabled zero values because no row existed in the database.
Changes:
- New
SeedDefaultSecurityConfigfunction using GORMFirstOrCreateto idempotently ensure a default row exists - Wired into
RegisterWithDepsstartup sequence afterAutoMigrate, with non-fatal error handling - Three unit tests covering empty-DB creation, idempotency, and preservation of existing data
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| backend/internal/models/seed.go | New function to seed default SecurityConfig row with safe disabled-by-default values |
| backend/internal/models/seed_test.go | Unit tests for empty DB, idempotency, and no-overwrite scenarios |
| backend/internal/api/routes/routes.go | Calls seed function after AutoMigrate in startup sequence |
You can also share your feedback on Copilot code review. Take the survey.
Contributor
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
Contributor
✅ Supply Chain Verification Results✅ PASSED 📦 SBOM Summary
🔍 Vulnerability Scan
📎 Artifacts
Generated by Supply Chain Verification workflow • View Details |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
- Added HTTP status checks for login and security config POST requests to ensure proper error handling. - Implemented a readiness gate for the Caddy admin API before applying security configurations. - Increased sleep duration before verifying rate limit handler to accommodate Caddy's configuration propagation. - Changed verification failure from a warning to a hard exit to prevent misleading test results. - Updated Caddy admin API URL to use the canonical trailing slash in multiple locations. - Adjusted retry parameters for rate limit verification to reduce polling noise. - Removed stale GeoIP checksum validation from the Dockerfile's non-CI path to simplify the build process.
…ty config - The rate-limit integration test was sending rate_limit_enable:true in the security config POST, but the backend injects the Caddy rate_limit handler only when rate_limit_mode is the string "enabled" - Because rate_limit_mode was absent from the payload, the database default of "disabled" persisted and the guard condition always evaluated false, leaving the handler uninjected across all 10 verify attempts - Replaced the boolean rate_limit_enable with the string field rate_limit_mode:"enabled" to match the exact contract the backend enforces
… test payload - The security config Upsert update path copied all rate limit fields from the incoming request onto the existing database record except RateLimitMode, so the seeded default value of "disabled" always survived a POST regardless of what the caller sent - This silently prevented the Caddy rate_limit handler from being injected on any container with a pre-existing config record (i.e., every real deployment and every CI run after migration) - Added the missing field assignment so RateLimitMode is correctly persisted on update alongside all other rate limit settings - Integration test payload now also sends rate_limit_enable alongside rate_limit_mode so the handler sync logic fires via its explicit first branch, providing belt-and-suspenders correctness independent of which path the caller uses to express intent
…n-major-updates chore(deps): update paulhatch/semantic-version action to v6.0.2 (feature/beta-release)
…on test
- All unquoted $i loop counter comparisons and ${TMP_COOKIE} curl
option arguments in the rate limit integration script were flagged
by shellcheck SC2086
- Unquoted variables in [ ] test expressions and curl -b/-c options
can cause subtle failures if the value ever contains whitespace or
glob characters, and are a shellcheck hard warning that blocks CI
linting gates
- Quoted all affected variables in place with no logic changes
…cert cleanup loop - The DB error return branch in SeedDefaultSecurityConfig was never exercised because all seed tests only ran against a healthy in-memory database; added a test that closes the underlying connection before calling the function so the FirstOrCreate error path is reached - The letsencrypt certificate cleanup loop in Register was unreachable in all existing tests because no test pre-seeded a ProxyHost with an letsencrypt cert association; added a test that creates that precondition so the log and Update lines inside the loop execute - These were the last two files blocking patch coverage on PR #852
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.
Summary
On a fresh install the
security_configstable is auto-migrated by GORM but contains no rows. Any code path readingSecurityConfigby name received an empty Go struct with zero values, producing an all-disabled UI state that offered no guidance to the user and made the security status endpoint appear broken before the user had intentionally disabled anything.Root Cause
GORM
AutoMigratecreates the table schema but does not insert default rows. TheGetStatushandler queriesWHERE name = 'default'— on a fresh database that row does not exist, so the handler falls back to static config defaults and returns a confusing all-empty state.Changes
SeedDefaultSecurityConfig— usesFirstOrCreateto guarantee a defaultSecurityConfigrow (name="default") exists on every startup with safe, disabled-by-default valuesAutoMigrateinRegisterWithDepsSecurity
CrowdSecAPIURLdefault ishttp://127.0.0.1:8085— loopback only, consistent with every other default reference in the codebaseTesting
Related
Part of the fresh-install bug investigation series. Addresses Issue #1 from the community bug report: "some database missing values — i think before crowdsec enabling."