fix(security): comprehensive auth and config hardening from audit - #189
Open
BillyOutlast wants to merge 230 commits into
Open
fix(security): comprehensive auth and config hardening from audit#189BillyOutlast wants to merge 230 commits into
BillyOutlast wants to merge 230 commits into
Conversation
* ci: add comprehensive CI workflow and SonarCloud configuration
- Add .github/workflows/ci.yml with actionlint validation, typecheck, lint, and test jobs
- Configure for both main and develop branches
- Add sonar-project.properties for SonarCloud analysis
- Set up coverage reporting and file exclusions
* chore: add dev-dependencies for cargo test harness
cli: add tempfile dev-dep
torrential: add tokio-test dev-dep
desktop: no change needed (tempfile already in deps)
* chore: add test dependencies (P1T1, P1T4)
Server: vitest, @nuxt/test-utils, @vue/test-utils, msw, @playwright/test,
@vitest/coverage-v8, happy-dom
Rust: tempfile (cli), tokio-test (torrential)
Part of TDD Wave 1.
Co-Authored-By: Sisyphus <sisyphus@opencode.ai>
* chore: format all Rust crates with cargo fmt (P4T2)
Formatting-only changes across cli, torrential, libraries/native_model,
desktop/src-tauri workspace. No logic changes.
Part of TDD Wave 1.
Co-Authored-By: Sisyphus <sisyphus@opencode.ai>
* chore: add .prettierignore to server/ (P4T1)
Exclude node_modules, .nuxt, .output, dist, .data, pnpm-lock.yaml
from prettier formatting.
Part of TDD Wave 1.
Co-Authored-By: Sisyphus <sisyphus@opencode.ai>
* chore: configure vitest workspace (P1T2)
- Add server/vitest.config.ts with @nuxt/test-utils
- Add server/test/setup.ts with msw lifecycle
- Add vitest.workspace.ts at repo root
- Add test, test:watch, coverage scripts to server/package.json
Part of TDD Wave 2.
Co-Authored-By: Sisyphus <sisyphus@opencode.ai>
* chore: configure Playwright E2E (P1T5)
- Add server/playwright.config.ts with baseURL, retries, webServer
- Add server/test/e2e/.gitkeep placeholder directory
- Add test:e2e script to server/package.json
Part of TDD Wave 2.
Co-Authored-By: Sisyphus <sisyphus@opencode.ai>
* test: add msw mocks for OIDC and metadata (P2T4)
- Add server/test/mocks/oidc.ts with configurable OIDC handlers
- Add server/test/mocks/jwt.ts with test JWT signing/verification
- Add server/test/mocks/metadata.ts with IGDB, Steam, Giantbomb mocks
- Add server/test/mocks/index.ts with setupTestMocks/teardownTestMocks lifecycle
Part of TDD Wave 2.
Co-Authored-By: Sisyphus <sisyphus@opencode.ai>
* ci: add format check to server-ci.yml (P4T3)
- Add format:check step before lint
- Separate format:check from lint:eslint for clarity
Part of TDD Wave 2.
Co-Authored-By: Sisyphus <sisyphus@opencode.ai>
* ci: add stale bot workflow (P5T2)
- Close issues inactive for 90 days
- 14-day warning before closure
- Exempt priority/p0 and priority/p1 labels
Part of TDD Wave 2.
Co-Authored-By: Sisyphus <sisyphus@opencode.ai>
* test: add health endpoint smoke test (P2T1)
- Create GET /api/v1/health endpoint returning { status, timestamp }
- Add smoke test verifying 200 response and shape
- Uses @nuxt/test-utils/e2e for integration testing
Part of TDD Wave 3.
Co-Authored-By: Sisyphus <sisyphus@opencode.ai>
* test: add Rust CLI tests (P2T3)
- Config tests: new, exists, get, get_active, serde roundtrip
- DepotManifest tests: new, append, overwrite, serde roundtrip, variants
- Uses tempfile for test isolation
Note: Tests require libarchive system library to compile.
Part of TDD Wave 3.
Co-Authored-By: Sisyphus <sisyphus@opencode.ai>
* chore: configure vitest coverage (P3T1)
- Provider: v8
- Reporters: text, lcov
- Reports directory: ./coverage
- Include: server/**/*.ts
- Exclude: test files and directories
Part of TDD Wave 3.
Co-Authored-By: Sisyphus <sisyphus@opencode.ai>
* chore: add pre-commit hooks (P4T4)
- Install husky and lint-staged
- Configure pre-commit hook to run lint-staged
- Lint-staged config: eslint --fix + prettier --write on *.{ts,vue}
- Prettier --write on *.json
Part of TDD Wave 3.
Co-Authored-By: Sisyphus <sisyphus@opencode.ai>
---------
Co-authored-by: John Smith <you@example.com>
Co-authored-by: BillyOutlast <billy@heretek.dev>
Co-authored-by: Sisyphus <sisyphus@opencode.ai>
…end lint-staged patterns
… health test handler arg
…revert manifest test changes
…on-error, codecov, root-config paths) (#163) * fix(ci): add develop to pull_request triggers + root-config paths to per-workspace CIs Wave 2 of CI remediation (issues #159, #161). - ci.yml, server-ci.yml, cli-ci.yml, desktop-ci.yml, droplet-ci.yml, e2e.yml, codeql.yml, osv-scanner.yml, editorconfig-ci.yml, open-code-review.yml: add 'develop' to pull_request.branches so PRs targeting develop fire CI. - cli-ci.yml, desktop-ci.yml, droplet-ci.yml: add pnpm-workspace.yaml, package.json, pnpm-lock.yaml to paths: filters so root-config changes trigger per-workspace CIs. Push triggers remain on rebuild only to avoid double-runs on merge. * fix(ci): unmask SonarCloud scan, add new-vuln detection, codecov config Wave 3 of CI remediation (issues #160, #161). - ci.yml SonarQube Scan step: remove continue-on-error: true so real scanner failures (e.g. auth errors) fail the build. Branch protection added in #158 requires this check. - ci.yml pnpm audit: keep continue-on-error but add follow-up step that fails only on NEW (un-ignored) critical advisories. - rust-ci/action.yml cargo audit: same pattern — keep continue-on-error, add follow-up step that fails only on new advisories not in security/risk-register.yaml. - osv-scanner.yml: keep continue-on-error: true on scan-scheduled (would fail on any transitive CVE) and document the rationale inline. - .github/codecov.yml: new file with informational thresholds. Project default threshold 2%, patch target 80%, both informational: true pending coverage milestone. fail_ci_if_error remains false in 3 locations to avoid blocking on Codecov infrastructure issues. * fix(ci): codecov flag_management schema + gate comment script on token - .github/codecov.yml: replace deprecated 'flags:' block with modern 'flag_management: default_rules + individual_flags' schema. Old schema was silently ignored causing 'configuration not found' warning. - .github/workflows/ci.yml: gate Post coverage gaps to PR on env.CODECOV_TOKEN != '' so missing secret no longer hard-fails the job (FATAL: CODECOV_TOKEN is not set surfaced after un-rubber-stamping). * fix(ci): extract new-vuln check into shared script with defensive JSON handling Address code review feedback on PR #163. - scripts/check-new-vulns.cjs (new): shared helper that parses pnpm and cargo audit JSON, compares against security/risk-register.yaml, and exits 1 only on confirmed new (un-accepted) advisories. - Defensive: missing/empty/malformed audit JSON exits 0 with a warning so transient tool failures no longer break the workflow. - Risk register parsing uses explicit 'advisory:' field line-scanning (no regex), supports both GHSA-* and RUSTSEC-* IDs (was GHSA-only). - .github/workflows/ci.yml: replace inline Node with script invocation. Adds --ignored GHSA-mp2f-45pm-3cg9 to match existing pnpm audit --ignore. - .github/actions/rust-ci/action.yml: replace inline Node with script. fallow-ignore-next-line on parseArgs/main: small CLI dispatcher + entry point; cyclomatic branches come from the CLI parser and JSON extraction paths. Refactoring would harm readability vs the simple suppression. Both functions are independently testable (run the script locally to exercise them). * fix(ci): resolve OpenCodeReview suggestions r3660539057/9071/9078/9607/9614/9624/9635 - scripts/check-new-vulns.cjs: default register path now uses GITHUB_WORKSPACE (env) with cwd fallback. Fixes r3660539607 where rust-ci composite action sets working-directory to cli/, desktop/src-tauri/, etc. — process.cwd() would have missed the register and flagged every Rust advisory as new. Verified via local smoke test with synthetic GITHUB_WORKSPACE=repo root and working-directory=subdir. - .github/workflows/cli-ci.yml, desktop-ci.yml, droplet-ci.yml: remove pnpm-lock.yaml from path triggers. Lockfile changes are JS-only and don't affect Rust builds (resolves r3660539614, r3660539635). Keep pnpm-workspace.yaml + package.json because those define workspace boundaries that can affect cargo resolver behavior. codecov.yml: already addressed in 171c267 — flag_management schema is the correct modern v4 form. The reviewer's flags: map suggestion targets the deprecated schema. RUSTSEC format support: already addressed in 171c267 — script uses explicit 'advisory:' field line-scan, not GHSA-only regex. JSON parse hardening: already addressed in 171c267 — script handles missing/empty/malformed JSON gracefully. * fix(ci): suppress fallow complexity on extractCargo arrow function * fix(ci): resolve script path bug, sonar cascade, help text, severity rank Per OpenCodeReview reviews 4791154356, 4791154664, 4791192875 (and the related failing CI checks on PR #163): - .github/actions/rust-ci/action.yml: use $GITHUB_WORKSPACE to locate scripts/check-new-vulns.cjs. The composite action is invoked with working-directory set to a sub-crate (cli/, desktop/src-tauri/, libraries/droplet/), where a relative 'scripts/check-new-vulns.cjs' would not exist. This is the root cause of the CLI CI / Desktop CI / Droplet CI failures on PR #163 (MODULES_NOT_FOUND). - .github/workflows/ci.yml: sonar-pr-comment now guards on 'needs.sonar.result == "success"', matching the existing guard on sonar-sync. Previously the job would still try to query the API for findings when the scan itself had failed, posting confusing empty comments. - scripts/check-new-vulns.cjs: --help now prints a dedicated HELP_TEXT constant instead of reading the first 25 lines of the source file (fragile to comment edits). - scripts/check-new-vulns.cjs: severityRank() now returns 4 (critical) for unrecognized or missing severity strings instead of -1. The previous behavior silently filtered out advisories with missing severity strings — a real false-negative risk. * fix(ci): suppress fallow complexity on extractCargo arrow function --------- Co-authored-by: John Smith <you@example.com>
CRITICAL fixes: - Session cookie: Added httpOnly, secure, sameSite, path attributes - Rate limiting: Enabled with 10 tokens per 60s interval - Password: Added maxLength: 128 validation - OIDC: Added redirect URL validation against origin HIGH fixes: - Mass assignment: Added field whitelists for company/game PATCH endpoints - TOTP: Replaced direct comparison with timing-safe hash comparison - File upload: Added MIME type whitelist and 10MB size limit Config hardening: - HSTS: Enabled with 1 year maxAge and includeSubdomains - Request size: Enabled 10MB limit for requests and uploads - NGINX: Added X-Frame-Options, X-Content-Type-Options, Referrer-Policy - Dockerfile: Added USER directive for non-root execution
- Use crypto.timingSafeEqual directly instead of SHA-256 hash comparison - Remove application/octet-stream from MIME allowlist - Make secure cookie flag dynamic based on protocol - Add body validation for admin PATCH endpoints - Add X-Forwarded-For header in nginx for rate limiter - Add client_max_body_size in nginx for request size limiting - Remove duplicate security headers (nuxt-security handles them) - Create /data and /library directories before USER node in Dockerfile
- Extract file validation into separate function (complexity 16 -> 15) - Use Set instead of Array for allowedFields in admin PATCH endpoints
- Fix Prisma field names in company/game PATCH whitelists (mName, mDescription, etc.) - Add max password length (128) to signup validator to prevent lockout - Increase rate limiter to 30 tokens/minute for admin workflows - Remove SVG from allowed MIME types (XSS risk) - Cache getRequestURL result in OIDC callback - Add /app and /pnpm to chown in Dockerfile for node user
- handlefileupload: MIME validation, size limits, file count - mass-assignment: field whitelisting, body validation - totp-timing: timingSafeEqual comparison - oidc-redirect: same-origin validation - session-cookie: security attributes
- Move proxy headers to location block in nginx.conf - Fix password validator format inconsistency in signup - Add mLogoObjectId and mBannerObjectId to company whitelist - Rename restOfTheBody to sanitizedData for clarity - Increase maxUploadFileRequestInBytes to 12MB for multipart overhead - Fix test file lint issues
- Lowercase MIME type before checking allowlist - Restore JSDoc comment for handleFileUpload - Fix error message to use MiB instead of MB
- Extract validateAndSanitizeBody helper to reduce code duplication - Add empty sanitizedData check in admin PATCH endpoints - Wrap OIDC URL parsing in try-catch for malformed URLs - Fix maxUploadFileRequestInBytes to match maxRequestSizeInBytes
- Use spaces around operators in password validator for consistency - Only chown /data and /library, not /app and /pnpm for security
Moved validateAndSanitizeBody from api/v1/admin/_helpers to server/internal/validation/body for proper separation of concerns.
…ests to invoke production handlers
…ests to invoke production handlers
Add X-Content-Type-Options, X-Frame-Options, Referrer-Policy, and Permissions-Policy headers to nginx reverse proxy configuration for defense-in-depth security hardening.
…pload Magic byte detection is authoritative. When file-type-mime cannot determine the MIME type from content, reject the file instead of falling back to the client-provided Content-Type header, which can be spoofed. Fixes bypass vector: attacker sends unrecognizable binary data with spoofed Content-Type (e.g. image/jpeg) to bypass upload validation.
Tests now use real JPEG/PDF magic bytes since the validateFile function requires content-based MIME detection and no longer falls back to client-provided Content-Type headers.
- File: server/server/plugins/08.auth-rate-limit.ts - Issue: No rate limiting on auth endpoints - Fix: Added in-memory rate limiter (5 req/60s per IP) for signin/signup - Proof: pnpm --filter drop test passes Closes #TBD
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
|
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.
Security Audit Fixes
Comprehensive security fixes from audit findings. All changes pass typecheck, lint, and tests.
Critical Fixes
/auth/signinand/auth/signupendpointsserver/server/plugins/08.auth-rate-limit.tsHigh Severity Fixes
file-type-mimeserver/server/internal/utils/handlefileupload.tsserver/test/unit/security/handlefileupload.test.tsConfiguration Hardening
server/build/nginx.confVerification
pnpm --filter drop typecheckpassespnpm --filter drop lintpassespnpm --filter drop testpasses (225 tests)Related
Closes #TBD