Code quality & security audit: extract magic numbers, add structured audit report

[Copilot]

Requested a full code-quality and security audit against architecture, coding style, comment standards, dead-code, and a three-dimensional vulnerability scan (network / low-level runtime / business logic), with results in a mandated structured report format.

Audit outcome

• No glue code, god classes, circular dependencies, or floating package versions found.
• Existing security posture is solid: BCrypt with timing-equalized unknown-user login, CryptographicOperations.FixedTimeEquals for API key comparison, realpath-based symlink-safe path resolution, parameterized SQL, global capability-based authorization filter, and pinned dependencies with a documented CVE mitigation for SQLitePCLRaw.
• One tracked medium-severity recommendation (not fixed here): dashboard JWT/payload stored in localStorage, exposing it to exfiltration if an XSS bug is ever introduced elsewhere. Remediating this requires a broader auth-transport redesign (HttpOnly cookie + CSRF token) that's out of scope for a minimal, low-risk fix — documented as a medium-term follow-up instead.

Fixes applied

• Extracted 4 magic numbers into named constants (behavior-preserving):
  • IdempotencyMiddleware: request-body hashing copy buffer size → RequestBodyCopyBufferBytes
  • OtaUpdateService: download streaming copy buffer size → DownloadCopyBufferBytes
  • FortOS.Cli/Program.cs: banner display delay → BannerDisplayDelayMilliseconds
  • FilePathResolver: realpath subprocess timeout → RealpathTimeoutSeconds

// before
var buffer = new byte[81920];

// after
private const int RequestBodyCopyBufferBytes = 81920;
...
var buffer = new byte[RequestBodyCopyBufferBytes];

Documentation

• Added docs/CODE_AUDIT_REPORT.md containing the full audit in the fixed format requested: quality score, architecture/style/dead-code findings, severity-classified vulnerability list, refactoring roadmap, and a qualified-code acceptance checklist.

• Fixes Full Code Quality & Security Audit Request: Architecture, Coding Style, Comment Standard, 3-Dimension Vulnerability Inspection #22

[JusterZhu]

@copilot Please complete the inspection and fix of all items mentioned in the issue one by one.

[Copilot]

@copilot Please complete the inspection and fix of all items mentioned in the issue one by one.

Went through each open item from the audit and turned the previously-documented recommendations into actual fixes (commit 47ab6b5):

• Added SecurityHeadersMiddleware (X-Content-Type-Options, X-Frame-Options, Referrer-Policy, Permissions-Policy) as defense-in-depth for the localStorage-token exposure, with a new integration test verifying the headers.
• Added debug-level logging to the three previously-silent "best-effort" catch blocks (ShareGrpcService, AiAssistantService SSE parsing, FilePathResolver realpath fallback) so the intentional fail-open behavior is now observable.
• CodeQL flagged a log-forging issue in the new FilePathResolver logging (user-supplied path written raw to logs); fixed by sanitizing control characters before logging.
• Updated docs/CODE_AUDIT_REPORT.md to reflect all fixes and raised the score from 88 to 92.

The one remaining open item — dashboard JWT stored in localStorage — still requires a full auth-transport redesign (HttpOnly cookie + CSRF token, on both backend and frontend) with no frontend test harness available to verify it safely, so it's kept as a documented follow-up rather than an unverified change; its blast radius is now reduced by the new security headers. Full solution builds clean and all 182 integration tests pass.

[Copilot]

Pull request overview

This PR implements outcomes from a requested code-quality & security audit by extracting a few repeated “magic numbers” into named constants, adding defense-in-depth HTTP security headers at the API layer, and documenting the audit results in a structured report.

Changes:

• Added SecurityHeadersMiddleware and wired it into the ASP.NET Core pipeline; added an integration test asserting the headers are present.
• Extracted several magic numbers into named constants across API middleware, OTA update streaming, CLI banner delay, and realpath timeout.
• Improved observability by adding debug logging in previously silent exception paths (AI SSE parsing, share-event JSON parsing, realpath fallback) and added docs/CODE_AUDIT_REPORT.md.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
tests/FortOS.Tests.Integration/Api/ApiGatewayTests.cs	Adds an integration test for security response headers.
src/FortOS.Modules.Update/Services/OtaUpdateService.cs	Extracts download streaming buffer size to a named constant.
src/FortOS.Modules.Share/Services/FilePathResolver.cs	Extracts realpath timeout, adds debug logging with log-forging sanitization helper.
src/FortOS.Cli/Program.cs	Extracts CLI banner delay to a named constant.
src/FortOS.Api/Services/AiAssistantService.cs	Adds optional logger injection and debug logging for skipped SSE JSON lines.
src/FortOS.Api/Program.cs	Registers the new security headers middleware in the request pipeline.
src/FortOS.Api/Middleware/SecurityHeadersMiddleware.cs	New middleware that attaches standard security headers to responses.
src/FortOS.Api/Middleware/IdempotencyMiddleware.cs	Extracts request-body copy buffer size to a named constant.
src/FortOS.Api/Grpc/ShareGrpcService.cs	Adds debug logging when share client event payload JSON parsing fails.
docs/CODE_AUDIT_REPORT.md	Adds the structured audit report documenting findings and recommendations.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.