-
Notifications
You must be signed in to change notification settings - Fork 2
Security Audit
This page documents all security findings for Cornerstone, maintained by the security-engineer agent. Findings are organized by implementation phase.
OWASP Category: A07 — Identification & Authentication Failures Severity: Medium Status: Resolved Date Found: 2026-02-16 Date Resolved: 2026-03-13
Description:
The /api/auth/login, /api/auth/setup, and password-change endpoints have no rate limiting. An attacker could attempt credential stuffing or brute-force attacks at unlimited speed.
Affected Files:
-
server/src/routes/auth.ts— no rate limit preHandler on login and setup routes
Proof of Concept:
Send unlimited POST requests to /api/auth/login with varying passwords; no throttling occurs.
Remediation:
Install @fastify/rate-limit and apply a preHandler on auth endpoints:
await app.register(import('@fastify/rate-limit'), {
max: 10,
timeWindow: '1 minute',
keyGenerator: (req) => req.ip,
});Apply with { config: { rateLimit: { max: 5, timeWindow: '1 minute' } } } on login and setup routes.
Risk if Unaddressed: Credential brute-force is technically possible, though mitigated by bcrypt/Argon2 cost and the self-hosted, low-exposure deployment model.
OWASP Category: A05 — Security Misconfiguration Severity: Low Status: Resolved Date Found: 2026-02-16 Date Resolved: 2026-03-13
Description:
No Content-Security-Policy, Strict-Transport-Security, X-Frame-Options, or X-Content-Type-Options headers are set. These are defense-in-depth headers.
Affected Files:
-
server/src/app.ts— no helmet-equivalent plugin registered
Remediation:
Install @fastify/helmet:
await app.register(import('@fastify/helmet'), {
contentSecurityPolicy: {
directives: {
defaultSrc: ["'self'"],
scriptSrc: ["'self'"],
styleSrc: ["'self'", "'unsafe-inline'"],
imgSrc: ["'self'", 'data:'],
},
},
});Risk if Unaddressed: Clickjacking and MIME-sniffing risks remain. Low impact given self-hosted deployment.
OWASP Category: A07 — Identification & Authentication Failures Severity: Low Status: Resolved Date Found: 2026-02-16 Date Resolved: 2026-03-13
Description: No account lockout after repeated failed login attempts. Combined with the rate limiting gap, this allows unlimited password attempts per account.
Affected Files:
-
server/src/services/userService.ts— login function has no failed-attempt counter
Remediation: Track failed login attempts in a server-side store (in-memory Map or DB table). After N failures (e.g., 10), require a cooldown period or admin unlock.
Risk if Unaddressed: Brute-force remains theoretically possible. Argon2id hashing significantly limits attack throughput.
OWASP Category: A04 — Insecure Design Severity: Low Status: Resolved Date Found: 2026-02-20 Date Resolved: 2026-03-13 PR: #150
Description:
The budget_categories.name TEXT UNIQUE NOT NULL DB constraint is case-sensitive (SQLite default), while the application service enforces case-insensitive uniqueness via a LOWER() comparison. A direct database insert (bypassing the application layer) could create duplicate category names differing only by case.
Affected Files:
-
server/src/db/migrations/0003_budget_categories.sql—name TEXT UNIQUE NOT NULL -
server/src/services/budgetCategoryService.ts—LOWER()uniqueness check
Remediation: Change the DB constraint to use a case-insensitive collation:
name TEXT UNIQUE NOT NULL COLLATE NOCASERisk if Unaddressed: Very low. Only reachable via direct DB access, which requires physical server access.
OWASP Category: A05 — Security Misconfiguration Severity: Low Status: Resolved Date Found: 2026-02-20 Date Resolved: 2026-03-13 PRs: #150, #151, #152, #187
Description:
The 409 conflict error responses from CATEGORY_IN_USE, VENDOR_IN_USE, BUDGET_SOURCE_IN_USE, and BUDGET_LINE_IN_USE errors include a details field exposing internal relationship counts (e.g., { invoiceCount: 3, budgetLineCount: 1 }). These counts are visible to authenticated clients in the raw API response, though not surfaced in the UI.
Affected Files:
-
server/src/errors/AppError.ts— allInUseerror constructors
Proof of Concept: Attempt to delete a category/vendor/source in use; the response body includes counts.
Remediation:
Remove the details field from InUse error constructors, or strip it before serialization. The details field is intended for debugging but should not expose internal data counts to API consumers.
Risk if Unaddressed: Information disclosure to authenticated users. Low risk given authenticated-only access and self-hosted model.
OWASP Category: A03 — Injection (stored XSS vector, low severity) Severity: Low Status: Resolved Date Found: 2026-02-20 Date Resolved: 2026-03-13 PR: #151
Description:
The email field on vendor create/update has no format validation — the AJV schema does not set format: 'email' and the service layer has no regex. Any string is accepted and stored. XSS is mitigated by React's auto-escaping, but malformed data could be stored.
Affected Files:
-
server/src/routes/vendors.ts—email: { type: 'string' }lacks format constraint
Remediation:
Add format: 'email' to the AJV schema properties for email:
email: { type: ['string', 'null'], format: 'email', maxLength: 254 },Note: Fastify must have ajv.plugins configured with ajv-formats for format to be enforced.
Risk if Unaddressed: Malformed email data stored; no direct security exploit path.
OWASP Category: A03 — Injection (DoS vector) Severity: Low Status: Open Date Found: 2026-02-20 PRs: #151, #152
Description:
budget_sources.terms and budget_sources.notes text fields lack maxLength constraints in the AJV request schema. Similarly, vendors.notes and invoices.notes have AJV-level constraints but no DB-level TEXT length enforcement (SQLite TEXT is unlimited). These omissions allow very large strings to be stored.
Affected Files:
-
server/src/routes/budgetSources.ts—termsandnoteslack maxLength
Remediation:
Add maxLength to AJV schemas for all free-text fields: terms: maxLength 500, notes: maxLength 2000.
Risk if Unaddressed: Storage DoS if an authenticated user submits very large text. Low risk in self-hosted deployment.
OWASP Category: A01 — Broken Access Control (data integrity variant) Severity: Low Status: Open Date Found: 2026-02-21 PR: #187
Description:
When creating or updating an invoice with a workItemBudgetId, the service validates that the referenced budget line exists but does not verify any relationship between the budget line's work item and the invoice's vendor. An authenticated user can link an invoice from Vendor A to a budget line associated with a completely unrelated work item (with no connection to Vendor A). This produces confusing budget aggregation data.
Affected Files:
-
server/src/services/invoiceService.ts:131-139—createInvoiceFK validation -
server/src/services/invoiceService.ts:236-246—updateInvoiceFK validation
Proof of Concept:
- Create a work item with a budget line (budget line ID =
bl-1) - Create an unrelated vendor (vendor ID =
v-99) - POST
/api/vendors/v-99/invoiceswith{ workItemBudgetId: "bl-1", ... }— succeeds
Remediation: After verifying the budget line exists, optionally verify the budget line belongs to a work item that references the same vendor:
// Check vendor-budget line consistency
if (budgetLine.vendorId && budgetLine.vendorId !== vendorId) {
throw new ValidationError('Budget line belongs to a different vendor');
}Alternatively, accept as a design decision — the budget line's vendorId is optional (a budget line can exist without a vendor) so the cross-link may be intentionally flexible.
Risk if Unaddressed: Data integrity only. No confidentiality breach — all authenticated users can view all work items and invoices. Budget overview aggregation may show unexpected numbers.
OWASP Category: A09 — Security Logging & Monitoring Failures (secondary UX degradation) Severity: Low Status: Open Date Found: 2026-02-22
Description:
In the invoice create and edit modals, the fetchWorkItemBudgets call is fired as a void .then(...) chain with no .catch() handler. If the API call fails (network error, 401 session expiry, 500), budgetLinesLoading remains true indefinitely — permanently disabling the Budget Line dropdown with no user-visible error or recovery path. The user must close and reopen the modal to reset the state. Not a direct security vulnerability, but a silent failure that prevents UI error reporting.
Affected Files:
-
client/src/pages/VendorDetailPage/VendorDetailPage.tsx:1037-1040— create modal budget line fetch, no.catch() -
client/src/pages/VendorDetailPage/VendorDetailPage.tsx:1256-1259— edit modal budget line fetch, no.catch()
Proof of Concept: With an active session, open the Add Invoice or Edit Invoice modal, disconnect the network, then select a work item from the dropdown. The Budget Line field appears but stays in a perpetually disabled state with no error shown.
Remediation:
Add a .catch() handler that resets budgetLinesLoading(false) and sets an inline error state:
void fetchWorkItemBudgets(workItemId)
.then((lines) => {
setBudgetLines(lines);
setBudgetLinesLoading(false);
})
.catch(() => {
setBudgetLinesLoading(false);
// optionally: setBudgetLineError('Failed to load budget lines.');
});Risk if Unaddressed: Degraded UX — users cannot link invoices to budget lines if the network call fails. No confidentiality, integrity, or authorization risk.
OWASP Category: A05 — Security Misconfiguration (client-server contract mismatch) Severity: Low Status: Open Date Found: 2026-02-22
Description:
The VendorDetailPage fetches work items for the invoice modal with listWorkItems({ pageSize: 200 }). The server-side AJV schema for GET /api/work-items enforces maximum: 100 on pageSize (server/src/routes/workItems.ts:44), so Fastify returns a 400 validation error. The void .then(...) call has no .catch() branch, so this rejection is silently swallowed — workItems stays as [] and the "Link to Work Item" dropdown appears empty. The invoice budget-line linking feature is functionally broken: users cannot link any invoice to a work item.
Affected Files:
-
client/src/pages/VendorDetailPage/VendorDetailPage.tsx:127—listWorkItems({ pageSize: 200 }) -
server/src/routes/workItems.ts:44—pageSize: { type: 'integer', minimum: 1, maximum: 100 }
Proof of Concept: Open the Vendor Detail page, click "Add Invoice". The "Link to Work Item" dropdown shows only the "None" option, even with work items present in the project.
Remediation:
Change pageSize: 200 to pageSize: 100 (the server maximum). If more than 100 work items need to be selectable, the server limit should be raised with appropriate pagination support.
void listWorkItems({ pageSize: 100 }).then((res) => setWorkItems(res.items));Risk if Unaddressed: Invoice-to-budget-line linking is entirely non-functional. No security risk, but the feature is broken for all users.
Resolution:
PR #203 fixed the pageSize value to 100 and the new InvoicesPage uses pageSize: 100. Finding closed.
OWASP Category: A05 — Security Misconfiguration (best-practice gap) Severity: Informational Status: Open Date Found: 2026-02-23 PR: #203
Description:
The getInvoiceByIdSchema defined in server/src/routes/standaloneInvoices.ts does not include additionalProperties: false on the params schema. This is inconsistent with the listAllInvoicesSchema in the same file, which correctly sets additionalProperties: false on the querystring schema. While Fastify does not allow unknown path parameters to reach handler code (they are ignored), omitting this constraint is a defense-in-depth gap and an inconsistency in the codebase's validation style.
Affected Files:
-
server/src/routes/standaloneInvoices.ts:3317-3325—getInvoiceByIdSchemaparams lacksadditionalProperties: false
Proof of Concept: No exploitable vector. Path parameters cannot contain unexpected extra fields because they are positionally extracted by the router. This is purely a best-practice gap.
Remediation:
Add additionalProperties: false to the params schema:
const getInvoiceByIdSchema = {
params: {
type: 'object',
required: ['invoiceId'],
properties: {
invoiceId: { type: 'string' },
},
additionalProperties: false,
},
};Risk if Unaddressed: None. Informational best-practice recommendation only.
OWASP Category: A05 — Security Misconfiguration (best-practice gap) Severity: Low Status: Open Date Found: 2026-02-24 PR: #247
Description:
The Fastify JSON schema for the color field in both createMilestoneSchema and updateMilestoneSchema is typed as { type: ['string', 'null'] } with no pattern constraint. Hex color validation only happens inside milestoneService.ts via the HEX_COLOR_RE regex. This is functionally correct (the service throws ValidationError which maps to 400), but the omission means the API contract does not self-document the format constraint at the schema layer. This is consistent with the existing open finding "Missing server-side maxLength on text fields" — the same pattern of relying solely on service-layer validation.
Affected Files:
-
server/src/routes/milestones.ts:1819-1825—createMilestoneSchemabody,colorlackspattern -
server/src/routes/milestones.ts:1833-1840—updateMilestoneSchemabody,colorlackspattern
Proof of Concept:
No direct exploit. A client submitting color: "red" will receive a 400 from the service layer, not from Fastify schema validation. The rejection path is correct; the schema layer is simply less defensive.
Remediation:
Add a pattern constraint to the JSON schema for color:
color: { type: ['string', 'null'], pattern: '^#[0-9A-Fa-f]{6}$' },Risk if Unaddressed: None — service layer correctly validates and rejects invalid colors. Minor defense-in-depth gap.
OWASP Category: A05 — Security Misconfiguration (defense-in-depth gap) Severity: Informational Status: Open Date Found: 2026-02-24 PR: #247
Description:
The leadLagDays integer field on dependency create (POST /api/work-items/:id/dependencies) and update (PATCH /api/work-items/:id/dependencies/:predecessorId) accepts any integer value with no minimum or maximum constraint at the Fastify schema or service layer. A value like 999999999 would be silently stored and later consumed by Gantt scheduling calculations.
Affected Files:
-
server/src/routes/dependencies.ts:597—createDependencySchemabody:leadLagDays: { type: 'integer' } -
server/src/routes/dependencies.ts:614—updateDependencySchemabody:leadLagDays: { type: 'integer' }
Remediation: Add sensible bounds to the JSON schema (±10 years as a reasonable maximum for a construction project):
leadLagDays: { type: 'integer', minimum: -3650, maximum: 3650 },Risk if Unaddressed:
No direct security exploit. If future Gantt scheduling logic performs date arithmetic with an extreme leadLagDays value, integer overflow or unexpected scheduling results could occur. Informational only.
OWASP Category: A05 — Security Misconfiguration (information exposure) Severity: Informational Status: Open Date Found: 2026-02-24 PR: #248
Description:
When a circular dependency is detected in the graph, CircularDependencyError is thrown with a details: { cycle: result.cycleNodes } payload that is included in the 409 error response body. The cycleNodes array contains internal work item IDs (e.g., UUIDs of items involved in the cycle). For an authenticated user, this is not a significant disclosure — they can already fetch all work item IDs via GET /api/work-items. The risk is informational in this single-tenant, self-hosted deployment model.
Affected Files:
-
server/src/routes/schedule.ts:101-104—CircularDependencyErrorconstructed with{ cycle: result.cycleNodes } -
server/src/errors/AppError.ts:110-119—CircularDependencyErroraccepts{ cycle: string[] }details
Proof of Concept:
POST /api/schedule with { "mode": "full" } when a dependency cycle exists. The 409 response body includes:
{
"error": {
"code": "CIRCULAR_DEPENDENCY",
"message": "...",
"details": { "cycle": ["work-item-uuid-a", "work-item-uuid-b"] }
}
}Remediation:
No remediation needed in this context. The work item IDs exposed are already visible to any authenticated user through the standard GET /api/work-items endpoint. The cycle detail is useful for UI diagnostics. If a multi-tenant architecture is ever introduced, the details field should be reconsidered.
Risk if Unaddressed: Negligible in a single-tenant deployment. All authenticated users have equal read access to all work items.
OWASP Category: A05 — Security Misconfiguration (defense-in-depth gap) Severity: Informational Status: Open Date Found: 2026-02-24 PR: #248
Description:
The anchorWorkItemId field in the Fastify JSON schema is typed as { type: ['string', 'null'] } with no minLength constraint. An empty string "" passes JSON schema validation and reaches the route handler, where !anchorWorkItemId evaluates to true (empty string is falsy), so it is rejected with a 400 VALIDATION_ERROR — the correct outcome. However, the rejection happens at the service layer rather than at the schema boundary. This is the same defense-in-depth pattern noted in prior PRs for other fields.
Affected Files:
-
server/src/routes/schedule.ts:22—anchorWorkItemId: { type: ['string', 'null'] }lacksminLength: 1
Remediation:
Add minLength: 1 to the schema to reject empty strings at the Fastify validation layer rather than the handler layer:
anchorWorkItemId: { type: ['string', 'null'], minLength: 1 },Risk if Unaddressed:
No exploitable vector. The handler correctly rejects empty anchorWorkItemId via the !anchorWorkItemId check. Purely a defense-in-depth gap.
OWASP Category: A05 — Security Misconfiguration (defense-in-depth gap) Severity: Informational Status: Open Date Found: 2026-02-25 PR: #263
Description:
The workItemIds array introduced in createMilestoneSchema (POST /api/milestones) has no maxItems bound and no maxLength on individual string items. A caller can submit an arbitrarily large array or arbitrarily long strings. The service-layer loop in milestoneService.createMilestone processes each ID with a separate DB round-trip (SELECT + INSERT), creating server-side processing cost proportional to array length with no upper bound enforced at the schema layer. This is the same defense-in-depth gap documented for other fields in the existing "Missing server-side maxLength on text fields" finding.
All DB operations use Drizzle ORM parameterized queries — there is no SQL injection risk. This is purely a resource exhaustion concern.
Affected Files:
-
server/src/routes/milestones.ts—createMilestoneSchemabody:workItemIdsitems have nomaxLength; array has nomaxItems
Proof of Concept:
Submit POST /api/milestones with workItemIds containing thousands of entries or strings of arbitrary length. The request passes JSON schema validation and the service loop executes one DB query per entry.
Remediation:
Add maxItems and per-item maxLength to the JSON schema:
workItemIds: {
type: 'array',
items: { type: 'string', maxLength: 36 },
maxItems: 200,
},maxLength: 36 matches the UUID format used for work item IDs. maxItems: 200 is a generous but bounded limit well above realistic use.
Risk if Unaddressed:
Authenticated user could cause excessive DB load by submitting a very large workItemIds array. Low risk given the self-hosted, trusted-user deployment model. No confidentiality or integrity risk.
OWASP Category: A10 — Server-Side Request Forgery (SSRF) Severity: Medium Status: Resolved Date Found: 2026-03-01 Date Resolved: 2026-03-01 PR: #362
Description:
PAPERLESS_URL is accepted as a raw string with no URL validation in config.ts. Any string value, including file://, ftp://, or http://169.254.169.254/ (cloud IMDS), is accepted without check and later concatenated directly into all upstream fetch URLs in paperlessService.ts. An operator with .env write access could redirect all proxy requests to internal services, metadata endpoints, or non-HTTP schemes.
Affected Files:
-
server/src/plugins/config.ts:108—paperlessUrlaccepted without URL format/scheme validation -
server/src/services/paperlessService.ts:91—fetch(`${baseUrl}${path}`, ...)— unvalidated base URL
Proof of Concept:
Set PAPERLESS_URL=http://169.254.169.254/latest/meta-data/ in .env. Any authenticated user triggering GET /api/paperless/status causes the server to fetch from the AWS IMDS endpoint.
Remediation:
Add URL validation in loadConfig using the built-in URL constructor:
if (paperlessUrl) {
try {
const parsed = new URL(paperlessUrl);
if (!['http:', 'https:'].includes(parsed.protocol)) {
errors.push(`PAPERLESS_URL must use http or https scheme, got: ${parsed.protocol}`);
}
} catch {
errors.push(`PAPERLESS_URL is not a valid URL: ${paperlessUrl}`);
}
}Risk if Unaddressed:
Operator with .env access can redirect proxy traffic to internal services. Blast radius is limited by the self-hosted deployment model — the attacker must already have server-level access. Should still be fixed for defense-in-depth.
Resolution:
PR #362 (round 2): config.ts now parses PAPERLESS_URL with new URL() at startup and enforces a ['http:', 'https:'] scheme allowlist, collecting validation errors into the existing errors array before throwing. file://, ftp://, and other schemes are rejected at server boot. Seven new tests cover http/https acceptance, file/ftp rejection, invalid URL rejection, and disabled-by-default behavior.
OWASP Category: A05 — Security Misconfiguration (information exposure) Severity: Low Status: Resolved Date Found: 2026-03-01 Date Resolved: 2026-03-01 PR: #362
Description:
When Paperless-ngx is configured but unreachable, getStatus() returns the raw OS-level exception message in the error field of the 200 response body. This can expose internal hostnames, IP addresses, port numbers, or TLS error details (e.g., ECONNREFUSED connect ECONNREFUSED 10.0.0.5:8000) to any authenticated Cornerstone user.
Affected Files:
-
server/src/services/paperlessService.ts:262-263— rawerr.messagereturned in status response
Remediation: Return a sanitized string instead of the raw error message, or document as accepted risk given the self-hosted admin context:
return { configured: true, reachable: false, error: 'Cannot connect to Paperless-ngx' };Risk if Unaddressed: Internal network topology may be disclosed to authenticated users. Low impact in self-hosted single-tenant deployment.
Resolution:
PR #362 (round 2): sanitizeErrorMessage() added to paperlessService.ts. Two regex patterns redact IPv4 addresses (\d{1,3}.\d{1,3}.\d{1,3}.\d{1,3}(?::\d+)?) and hostname:port pairs, replacing with <host>. The error type (e.g., ECONNREFUSED, ENOTFOUND) is preserved. Four new tests verify IP and hostname redaction, non-network errors, and 403 probe failures.
OWASP Category: A05 — Security Misconfiguration Severity: Low Status: Resolved Date Found: 2026-03-01 Date Resolved: 2026-03-01 PR: #362
Description:
The thumb and preview endpoints forward the upstream content-type header from Paperless-ngx without any allowlist validation. If Paperless-ngx returns an unexpected content type such as image/svg+xml (which can contain embedded <script> elements), the browser may execute JavaScript in Cornerstone's origin when the image is rendered inline.
Affected Files:
-
server/src/routes/paperless.ts:165-173— thumb endpoint, unvalidated content-type passthrough -
server/src/routes/paperless.ts:202-210— preview endpoint, unvalidated content-type passthrough
Remediation: Allowlist permitted content types and fall back to a safe default:
const ALLOWED_THUMB_TYPES = new Set(['image/webp', 'image/jpeg', 'image/png', 'image/gif']);
const upstreamType = upstream.headers.get('content-type') ?? '';
const contentType = ALLOWED_THUMB_TYPES.has(upstreamType) ? upstreamType : 'image/webp';Risk if Unaddressed: Low in practice — Paperless-ngx is a trusted backend. A compromised or misconfigured Paperless instance could deliver SVG-based XSS through the Cornerstone proxy.
Resolution:
PR #362 (round 2): sanitizeBinaryContentType() helper added to paperless.ts. A BINARY_CONTENT_TYPE_ALLOWLIST Set (image/webp, image/png, image/jpeg, image/gif, application/pdf) gates all content-type passthrough for thumb and preview endpoints. Types outside the allowlist (including image/svg+xml, text/html, application/javascript) fall back to application/octet-stream. MIME parameters (e.g., image/webp; charset=utf-8) are stripped before matching. Tests verify allowlisted passthrough, disallowed-type fallback, and null-header default-type behavior.
OWASP Category: A03 — Injection (low severity) Severity: Low Status: Resolved Date Found: 2026-03-01 Date Resolved: 2026-03-01 PR: #362
Description:
The tags query parameter on GET /api/paperless/documents is typed as { type: 'string', maxLength: 200 } with no format constraint. Arbitrary strings are forwarded to Paperless-ngx via URLSearchParams (which correctly URL-encodes them, preventing header injection). Non-numeric values will cause Paperless-ngx to return an error surfaced as PAPERLESS_ERROR 502, potentially exposing upstream API error messages to the client.
Affected Files:
-
server/src/routes/paperless.ts:27—tags: { type: 'string', maxLength: 200 }lackspatternconstraint
Remediation: Add a pattern constraint to enforce comma-separated integers:
tags: { type: 'string', maxLength: 200, pattern: '^\\d+(,\\d+)*$' },Risk if Unaddressed:
URLSearchParams encoding prevents HTTP injection. Only risk is upstream error message leakage on invalid input. Low impact.
Resolution:
PR #362 (round 2): pattern: '^\\d+(,\\d+)*$' added to the tags property in listDocumentsSchema. Non-numeric and SQL-injection-style values now return 400 at the Fastify schema validation layer before reaching the service. Tests verify rejection of abc, 1 OR 1=1 (URL-encoded), and acceptance of valid comma-separated integers like 5,12,20.
OWASP Category: A05 — Security Misconfiguration (defense-in-depth gap) Severity: Informational Status: Open Date Found: 2026-02-26 PR: #308
Description:
The actualStartDate and actualEndDate fields added to createWorkItemSchema and updateWorkItemSchema both carry format: 'date' constraints, which correctly enforces ISO date format. However, there is no cross-field validation ensuring actualStartDate <= actualEndDate. A caller can submit actualStartDate: "2026-05-10" and actualEndDate: "2026-03-01" — an end date that precedes the start date — and this passes schema validation and is written to the database without error.
The existing startDate / endDate pair has the same gap (also noted as a carryover from prior PRs), so this is consistent with the project's existing validation style. The scheduling engine uses actualStartDate as ES and actualEndDate as EF, meaning an inverted actual-date pair would produce a negative-duration Gantt bar and potentially confuse CPM calculations for downstream dependencies.
Affected Files:
-
server/src/routes/workItems.ts:24-25—actualStartDate/actualEndDateschema lacks cross-field ordering constraint -
server/src/services/workItemService.ts:445-451— no ordering check before DB write
Proof of Concept:
PATCH /api/work-items/<id>
Content-Type: application/json
{ "actualStartDate": "2026-12-31", "actualEndDate": "2026-01-01" }Returns 200. Both values are persisted. The Gantt bar for this item would have a negative computed width.
Remediation:
Add a cross-field ordering check in workItemService.updateWorkItem and createWorkItem, analogous to the existing validateDateConstraints:
if (actualStartDate && actualEndDate && actualEndDate < actualStartDate) {
throw new ValidationError('actualEndDate must be on or after actualStartDate');
}Optionally, add this as a custom AJV keyword or an if/then JSON Schema constraint for defense-in-depth at the schema layer.
Risk if Unaddressed: No confidentiality or authorization risk. An authenticated user could store an inverted actual-date pair that produces nonsensical Gantt rendering and could push downstream scheduled items to unexpected positions. Low practical risk in this single-tenant, trusted-user deployment model.
OWASP Category: A01 — Broken Access Control (horizontal privilege escalation) Severity: Low Status: Open Date Found: 2026-03-02 PR: #363
Description:
DELETE /api/document-links/:id allows any authenticated user to delete any document link by its ID regardless of who created it. The createdBy field is persisted on the link but is not consulted during deletion. A member-role user who knows or guesses a link ID created by another user can delete it.
This finding requires a design decision. If the intent is that all authenticated users share equal write access to document links (consistent with how work items, budget lines, and invoices are managed in this single-tenant application), this is acceptable as an accepted design decision. If the intent is that only the link creator or an Admin can delete links, an ownership or role check must be added.
Affected Files:
-
server/src/routes/documentLinks.ts— delete handler callsdocumentLinkService.deleteLink(fastify.db, request.params.id)with no ownership or role check -
server/src/services/documentLinkService.ts—deleteLink()performs no ownership validation
Proof of Concept:
- User A creates a document link → receives link ID
link-abc - User B (any authenticated user) sends
DELETE /api/document-links/link-abc→ 204 No Content, link deleted
Remediation: If ownership restriction is desired, add a creator or role check before deletion:
// In the delete handler:
const existing = documentLinkService.getLinkById(fastify.db, request.params.id);
if (!existing) throw new NotFoundError('Document link not found');
if (existing.createdBy?.id !== request.user.id && request.user.role !== 'admin') {
throw new ForbiddenError('Cannot delete a link created by another user');
}
documentLinkService.deleteLink(fastify.db, request.params.id);If the design intent is equal access (consistent with rest of the application), document this in ADR-015 and close this finding as Accepted Risk.
Risk if Unaddressed: Any authenticated user can delete any document link created by any other user. In this single-tenant, trusted-user deployment model the blast radius is low — there is no cross-tenant data leakage, only potential collaborative disruption between users who already share all data.
OWASP Category: A05 — Security Misconfiguration (defense-in-depth gap) Severity: Informational Status: Open Date Found: 2026-03-02 PR: #363
Description:
The deleteLinkSchema for DELETE /api/document-links/:id does not set additionalProperties: false on the params schema, inconsistent with createLinkSchema and listLinksSchema in the same file which correctly set additionalProperties: false. This is the same pattern noted in the existing finding for getInvoiceByIdSchema. Path parameters cannot contain unexpected extra fields in Fastify's positional router so there is no exploitable vector.
Affected Files:
-
server/src/routes/documentLinks.ts—deleteLinkSchema.paramslacksadditionalProperties: false
Remediation:
const deleteLinkSchema = {
params: {
type: 'object',
required: ['id'],
properties: {
id: { type: 'string' },
},
additionalProperties: false,
},
};Risk if Unaddressed: None. Informational best-practice recommendation only.
OWASP Category: A05 — Security Misconfiguration (defense-in-depth gap) Severity: Informational Status: Open Date Found: 2026-03-02 PR: #363
Description:
The entityId field in both createLinkSchema (body) and listLinksSchema (querystring) sets minLength: 1 but no maxLength. Entity IDs are UUIDs (36 characters) in practice. All DB operations use Drizzle ORM parameterized queries so there is no injection risk; SQLite TEXT columns are unbounded. This is consistent with the existing open finding "Missing server-side maxLength on text fields".
Affected Files:
-
server/src/routes/documentLinks.ts—createLinkSchema.body.properties.entityIdandlistLinksSchema.querystring.properties.entityIdlackmaxLength
Remediation:
Add maxLength: 36 to both entityId schema definitions to match the UUID format used throughout the application.
Risk if Unaddressed:
An authenticated user could store an oversized string in the entity_id column. No SQL injection or authorization risk. Low practical impact.
OWASP Category: A03 — Injection (XSS) Severity: Informational Status: Open Date Found: 2026-03-02 PR: #364
Description:
The PaperlessSearchHit.highlights field (type string) contains HTML markup with <em> tags generated by Paperless-ngx's full-text search engine. This field is present in the PaperlessDocumentSearchResult type and is available in the data model flowing through the Document Browser. In PR #364 (Story 8.3), this field is not rendered — no component accesses searchHit.highlights. However, if a future story adds rendering of this field to highlight matched search terms, using dangerouslySetInnerHTML without a strict HTML sanitizer would introduce a Stored/Reflected XSS vulnerability, as Paperless-ngx could return arbitrary HTML depending on document content and the Paperless-ngx version.
Affected Files:
-
shared/src/types/document.ts:70—PaperlessSearchHit.highlightstyped asstring(HTML content) -
client/src/components/documents/DocumentCard.tsx—searchHitfield present in document prop but not rendered (safe) -
client/src/components/documents/DocumentDetailPanel.tsx—searchHitfield present in document prop but not rendered (safe)
Proof of Concept: If a future implementation adds:
<p dangerouslySetInnerHTML={{ __html: document.searchHit.highlights }} />...and Paperless-ngx returns <em onclick="alert(1)">match</em> or a crafted payload, XSS would execute in the user's browser.
Remediation:
If searchHit.highlights is rendered in a future story, one of the following approaches must be used:
-
Server-side stripping (preferred): In the Paperless-ngx proxy service (
server/src/services/paperlessService.ts), strip all HTML tags fromhighlightsbefore forwarding to the client, or allow only<em>with no attributes via a server-side allowlist. The client then renders plain text (or uses CSS-only highlighting). -
Client-side sanitization with DOMPurify: If HTML rendering is required for visual highlighting, use
DOMPurify.sanitize(highlights, { ALLOWED_TAGS: ['em'], ALLOWED_ATTR: [] })before passing todangerouslySetInnerHTML. DOMPurify must be added as a dependency and the configuration must be strictly allowlisted.
Any PR that introduces rendering of this field must receive a security review before merge.
Risk if Unaddressed:
If highlights is rendered without sanitization, a malicious or compromised Paperless-ngx instance could inject arbitrary HTML/JavaScript into the Cornerstone UI, leading to session hijacking, credential theft, or UI redress attacks for all connected users.
OWASP Category: A05 — Security Misconfiguration (defense-in-depth gap) Severity: Informational Status: Open Date Found: 2026-03-03 PR: #402
Description:
The POST body schema for POST /api/household-items/:householdItemId/work-items validates workItemId as { type: 'string' } with no minLength: 1 constraint. An empty string "" passes schema validation and reaches the service layer, where assertWorkItemExists queries the database for a work item with id = "" and correctly returns a NotFoundError (404). There is no exploit path.
This is consistent with the existing codebase pattern — no route in the application specifies minLength on ID body fields. Noted for completeness.
Affected Files:
-
server/src/routes/householdItemWorkItems.ts:21—linkWorkItemSchema.body.properties.workItemIdlacksminLength: 1
Remediation:
workItemId: { type: 'string', minLength: 1 },Risk if Unaddressed: None. The service-layer existence check provides a correct 404 response for empty or non-existent IDs. No injection, authorization, or data integrity risk.
OWASP Category: A05 — Security Misconfiguration (defense-in-depth gap) Severity: Informational Status: Open Date Found: 2026-03-09 PR: #708
Description:
The upsertPreferenceSchema in server/src/routes/preferences.ts constrains the key field with minLength: 1, maxLength: 100, but the value field has no length constraint ({ type: 'string' }). An authenticated user could submit an arbitrarily large string value for any preference key and it would be stored and returned on every page load.
In the current deployment model (single-tenant SQLite), storage-level DoS is negligible. The primary risk is that future preference consumers may not re-validate the value before use. The ThemeContext.tsx consumer correctly validates the theme preference value against a closed enum ('light' | 'dark' | 'system') before applying it, which prevents XSS from a tampered value. However, the dashboard.hiddenCards preference (JSON array) and any future keys added by new stories rely on the absence of a size contract, which complicates downstream validation reasoning.
Affected Files:
-
server/src/routes/preferences.ts:24—value: { type: 'string' }with nomaxLength
Remediation:
Add a maxLength appropriate for the largest expected preference payload. For example, dashboard.hiddenCards is a JSON array of up to 8 card IDs (each ~30 characters), so maxLength: 10000 provides a generous bound with no practical impact on legitimate users:
value: { type: 'string', maxLength: 10000 },Risk if Unaddressed: No current exploit path. Future consumers that do not re-validate preference values before use may be vulnerable to oversized or malformed payloads.
OWASP Category: A05 — Security Misconfiguration (defense-in-depth gap) Severity: Informational Status: Open Date Found: 2026-03-09 PR: #708
Description:
The deletePreferenceSchema params schema declares key as { type: 'string' } with no length constraints, inconsistent with the PATCH body schema which correctly applies minLength: 1, maxLength: 100 to the same field. An empty-string key cannot match any stored record (keys are created non-empty via PATCH), so the result is always a 404. There is no exploitable vector.
Affected Files:
-
server/src/routes/preferences.ts:31—key: { type: 'string' }indeletePreferenceSchema.params(nominLength, nomaxLength)
Remediation: Mirror the PATCH schema constraints on the DELETE param:
key: { type: 'string', minLength: 1, maxLength: 100 },Risk if Unaddressed:
None. The preferencesService.deletePreference() query correctly filters by both userId (session-bound) and key, so an empty-string key returns false (not found) and a 404 response. No authorization, injection, or data integrity risk.
OWASP Category: A01 — Broken Access Control Severity: High Status: Resolved Date Found: 2026-03-13 Date Resolved: 2026-03-17
Description:
GET /feeds/cal.ics and GET /feeds/contacts.vcf are registered outside the authenticated route tree with no authentication, no secret token, and no network-level restriction. Any actor who can reach the server receives full work item titles and dates, milestone titles and completion dates, household item names and delivery dates, and all vendor contact details (name, email, phone, address, specialty, notes). The intent is calendar/contact subscription for household members, but there is no mechanism to limit which callers may subscribe.
Affected Files:
-
server/src/routes/feeds.ts:46—/cal.icshandler registered with no auth check -
server/src/routes/feeds.ts:111—/contacts.vcfhandler registered with no auth check -
server/src/app.ts:196— feeds plugin registered outside authenticated route group with nopreHandler
Proof of Concept:
curl http://<server>/feeds/cal.ics
curl http://<server>/feeds/contacts.vcf
Both return full data with HTTP 200 without any session cookie or token.
Remediation:
Generate a random 32-byte opaque token at first run, persist it in a settings table row, and require it as a query parameter. The URL becomes /feeds/cal.ics?token=<hex>. This is the model used by Fastmail, Google Calendar, and Proton Calendar for anonymous subscription URLs. Display the subscription URLs (with embedded token) in the application UI.
// Validate token in both handlers before processing
const { token } = request.query as { token?: string };
const expectedToken = getFeedToken(fastify.db); // read from settings table
if (!token || token !== expectedToken) {
return reply.status(401).send({ error: { code: 'UNAUTHORIZED', message: 'Invalid feed token' } });
}Risk if Unaddressed: In a cloud-hosted or port-forwarded deployment, any person who discovers the URL receives all project scheduling data and all vendor PII without authentication. This constitutes a data breach of business-critical information.
Resolution: PR #936 removed /feeds/* entirely and replaced the functionality with authenticated /dav/* endpoints requiring HTTP Basic Auth with a per-user DAV token (256-bit random, stored with partial unique index). All calendar and contact data is now gated behind davAuth.
OWASP Category: A02 — Cryptographic Failures / Sensitive Data Exposure Severity: Medium Status: Resolved Date Found: 2026-03-13 Date Resolved: 2026-03-17
Description:
Both feed endpoints set Cache-Control: public, max-age=3600. The public directive explicitly authorises shared caches (CDNs, reverse proxies, ISP transparent caches) to store and re-serve the response to any downstream requester. The feeds contain vendor PII and full project schedule data, making public caching inappropriate regardless of the intentional lack of auth.
Affected Files:
-
server/src/routes/feeds.ts:159—Cache-Control: public, max-age=3600on/cal.ics -
server/src/routes/feeds.ts:193—Cache-Control: public, max-age=3600on/contacts.vcf
Remediation:
Change to Cache-Control: private, max-age=3600. If a feed-token scheme is added (see HIGH finding above), private is mandatory — a CDN caching one token-bearer's response and returning it to a different caller would constitute a cache-poisoning vulnerability.
Risk if Unaddressed: Any CDN, reverse proxy, or ISP transparent cache between the client and server may cache the full vendor/schedule feed and serve it to unrelated callers.
Resolution: PR #936 removed /feeds/*. The replacement /dav/* endpoints do not set Cache-Control: public and serve authenticated content only.
OWASP Category: A02 — Data Integrity Severity: Low Status: Resolved Date Found: 2026-03-13 Date Resolved: 2026-03-17
Description:
Both feed handlers compute their ETag as the first 16 hex chars of SHA256(MAX(updated_at)). SQLite stores updated_at as text with second-level precision. Two updates within the same second produce the same ETag, causing the second caller to receive a stale 304 response. Additionally, a database restore to an earlier snapshot replays old updated_at values, producing the same ETag for different content.
Affected Files:
-
server/src/routes/feeds.ts:107— ETag computed fromMAX(updated_at)only for/cal.ics -
server/src/routes/feeds.ts:138— ETag computed fromMAX(updated_at)only for/contacts.vcf
Remediation: Include the row count in the ETag hash to cover same-second updates:
const etag = computeETag([String(rowCount), maxUpdatedAt]);Risk if Unaddressed: Stale schedule or vendor data delivered to calendar/contact apps after same-second updates or DB restores. No security exploit path; a data integrity concern.
Resolution: PR #936 removed /feeds/*. The new /dav/* CalDAV/CardDAV server uses SHA256(MAX(updated_at)).slice(0,16) which carries the same characteristic, but correctness of cache revalidation is a feature concern, not a security risk in the authenticated model.
OWASP Category: A07 — Identification & Authentication Failures Severity: Low Status: Open Date Found: 2026-03-17
Description:
The davAuth preHandler in dav.ts throws UnauthorizedError (HTTP 401) when Basic Auth credentials are absent or invalid, but does not set the WWW-Authenticate: Basic realm="Cornerstone DAV" header. RFC 7235 §4.1 requires this header on all 401 responses to Basic Auth challenges. Without it, CalDAV/CardDAV clients (Apple Calendar, Apple Contacts, Thunderbird) cannot correctly identify the authentication scheme and may silently fail, show confusing prompts, or fall back to unsupported schemes.
Affected Files:
-
server/src/routes/dav.ts—davAuthfunction does not setWWW-Authenticatebefore throwing
Proof of Concept:
curl -v http://<server>/dav/
# Response: HTTP/1.1 401
# Missing: WWW-Authenticate: Basic realm="Cornerstone DAV"
Remediation:
Add a scoped onError hook inside the davRoutes plugin that injects the required header for 401 responses:
fastify.addHook('onError', async (_request, reply, error) => {
if ((error as any).statusCode === 401) {
reply.header('WWW-Authenticate', 'Basic realm="Cornerstone DAV"');
}
});Risk if Unaddressed:
CalDAV/CardDAV clients that strictly follow RFC 7235 may fail to prompt for credentials or complete auto-configuration from the .mobileconfig profile, reducing the practical usability of the DAV server.
OWASP Category: A04 — Insecure Design (defense-in-depth gap) Severity: Informational Status: Open Date Found: 2026-05-10
Description:
The AJV body schemas in invoiceDeposits.ts declare dueDate, paidDate, and claimedDate as { type: 'string' } without a pattern or format: 'date' constraint. The service layer correctly validates ISO 8601 format via a regex and new Date() calendar check, so invalid dates are rejected. The schema layer does not reject them first.
Affected Files:
-
server/src/routes/invoiceDeposits.ts—createDepositSchemaandupdateDepositSchemadate string properties
Remediation:
Add pattern: '^\\d{4}-\\d{2}-\\d{2}$' to the date string properties in the AJV schemas to enforce format at the schema layer before service logic is invoked.
Risk if Unaddressed: No exploit path. Service-layer validation catches all invalid input. Schema-layer enforcement is a defence-in-depth improvement only.
OWASP Category: A04 — Insecure Design (defense-in-depth gap) Severity: Informational Status: Open Date Found: 2026-05-10
Description:
The AJV body schemas for creating and updating invoice deposits declare description as { type: ['string', 'null'] } without a maxLength constraint. The service layer enforces a 500-character limit and correctly rejects overlength input. Consistent with informational finding (preferences value field) from PR #708.
Affected Files:
-
server/src/routes/invoiceDeposits.ts—createDepositSchemaandupdateDepositSchemadescription property
Remediation:
Add maxLength: 500 to the description property in both schemas to make the constraint visible at the API contract layer and allow AJV to produce a schema-level 400 without entering service logic.
Risk if Unaddressed: No exploit path. Service-layer validation catches overlength input. Schema-layer enforcement is a defence-in-depth improvement only.
OWASP Category: A05 — Security Misconfiguration (minor) Severity: Informational Status: Open Date Found: 2026-05-10
Description:
listDepositsSchema in invoiceDeposits.ts defines only a params schema with no response schema. Fastify AJV serialisation cannot strip unexpected fields from the response without a response schema. assertInvoiceExists() correctly returns 404 for unknown invoice IDs so there is no access-control gap. Consistent with patterns noted in PR #203 and PR #363 reviews.
Affected Files:
-
server/src/routes/invoiceDeposits.ts—listDepositsSchemaobject (noresponsekey)
Remediation:
Add a response: { 200: { ... } } schema to listDepositsSchema describing the { deposits: InvoiceDeposit[] } shape.
Risk if Unaddressed: No exploit path. No unexpected fields are present in the current response. Remediation improves type safety at the serialisation layer only.
OWASP Category: A05 — Security Misconfiguration / A03 — Injection (file-upload validation gap) Severity: Low Status: Open Date Found: 2026-05-18 PR: Story #1478 audit
Description:
The PUT /api/photos/:id/annotation endpoint receives a multipart file upload and writes the buffer directly to disk as annotated.png without validating that the uploaded content is actually a PNG image. The original photo upload endpoint (POST /api/photos) validates MIME types via ALLOWED_MIME_TYPES, but this check is absent from the annotation endpoint. Sharp is called only for thumbnail generation — after the raw buffer is already written to disk.
Two concrete risks arise:
-
MIME spoofing: A client may upload a valid JPEG (or WebP) with
Content-Type: image/pngin the multipart part. Sharp'sresize().webp().toBuffer()call will process the JPEG successfully (it detects format from magic bytes). The result is saved asannotated.pngand served withContent-Type: image/png, which misidentifies the format. -
Non-image binary: If a client uploads garbage data (a PDF, an executable, a text file) the sharp thumbnail call will throw because libvips cannot decode the format. The raw buffer has already been written to
annotated.pngbywriteFile(annotatedPath, pngBuffer)before sharp throws. Fastify catches the uncaught rejection and the error handler returns 500 — but the invalid file remains on disk until the next successful annotation orDELETE /:id/annotationoverwrites it. The file is then served withContent-Type: image/pngto any viewer.
Affected Files:
-
server/src/routes/photos.ts:368-396—PUT /:id/annotationhandler: nofile.mimetypecheck -
server/src/services/photoAnnotationService.ts:47—writeFile(annotatedPath, pngBuffer)before any image validation
Proof of Concept:
# Upload a PDF masquerading as a PNG
curl -X PUT /api/photos/<valid-id>/annotation \
-H "Cookie: session=<valid-session>" \
-F "file=@document.pdf;type=image/png"
# Sharp fails on thumbnail generation → 500 response
# annotated.png (containing PDF bytes) is left on disk
# Subsequent GET /:id/file returns the PDF bytes with Content-Type: image/png
Remediation:
Add MIME-type validation in the route handler before calling the service, and restructure saveAnnotatedImage to write to disk only after sharp validates the buffer:
// In the PUT /:id/annotation route handler (photos.ts):
if (file.mimetype !== 'image/png') {
throw new ValidationError('Annotation must be a PNG image');
}Additionally, in saveAnnotatedImage (photoAnnotationService.ts), move writeFile(annotatedPath, pngBuffer) to occur after the sharp thumbnail call so that if sharp throws (unreadable binary), no partial file is left on disk:
// Validate and generate thumbnail first
const thumbnailBuffer = await sharp(pngBuffer)
.resize(300, 300, { fit: 'inside', withoutEnlargement: true })
.webp()
.toBuffer();
// Only write to disk after successful sharp processing
await writeFile(annotatedPath, pngBuffer);
await writeFile(thumbnailPath, thumbnailBuffer);Risk if Unaddressed:
In the self-hosted, authenticated single-instance model the blast radius is low: only authenticated users can reach this endpoint, and any user can already view all photos. The primary consequence is a corrupted annotated.png file that serves garbled bytes to the viewer. No privilege escalation or remote code execution path exists.
OWASP Category: A05 — Security Misconfiguration (HTTP semantics) Severity: Informational Status: Open Date Found: 2026-05-18 PR: Story #1478 audit
Description:
When an uploaded file exceeds PHOTO_MAX_FILE_SIZE_MB (default 20 MB), both the original upload handler (POST /api/photos) and the annotation handler (PUT /api/photos/:id/annotation) throw a ValidationError which maps to HTTP 400 Bad Request. The HTTP standard for payload-too-large is 413 Payload Too Large (RFC 9110 §15.5.14). Returning 400 is incorrect and can confuse CalDAV/HTTP clients that inspect status codes to distinguish input validation errors from size-limit rejections.
Affected Files:
-
server/src/routes/photos.ts:155-159— file size guard throwsValidationError(→ 400) -
server/src/routes/photos.ts:380-384— same pattern in annotation handler -
server/src/errors/AppError.ts— noPayloadTooLargeErrorclass exists
Remediation:
Introduce a PayloadTooLargeError that maps to 413:
export class PayloadTooLargeError extends AppError {
constructor(message = 'Payload too large') {
super('PAYLOAD_TOO_LARGE', 413, message);
this.name = 'PayloadTooLargeError';
}
}Replace the ValidationError in the file-size guards with PayloadTooLargeError.
Risk if Unaddressed: No security risk. Semantic HTTP correctness only.
OWASP Category: A05 — Security Misconfiguration Severity: Informational Status: Open Date Found: 2026-05-18 PR: Story #1478 audit
Description:
The @fastify/multipart plugin is registered with a hard fileSize limit of 50 MB (app.ts:91-95). When this limit is exceeded, @fastify/multipart throws FST_ERR_CTP_BODY_TOO_LARGE — a Fastify internal error with statusCode: 413. The application's errorHandler only handles AppError instances and AJV validation errors; all other errors fall through to the "unknown error" branch which returns 500 in production. So a request exceeding 50 MB receives a misleading 500 response rather than 413.
This issue affects all endpoints that use multipart parsing (photo upload and photo annotation).
Affected Files:
-
server/src/app.ts:91-95— multipart 50 MB limit registration -
server/src/plugins/errorHandler.ts:47-56— unknown-error branch does not inspecterror.statusCode
Remediation:
In errorHandler.ts, add a passthrough for Fastify's native errors that already carry a correct HTTP status code:
// Fastify built-in errors (e.g., FST_ERR_CTP_BODY_TOO_LARGE → 413)
if (error.statusCode && !error.validation) {
return reply.status(error.statusCode).send({
error: {
code: error.code ?? 'REQUEST_ERROR',
message: isProduction ? 'Request error' : error.message,
},
});
}Risk if Unaddressed: No security risk. 500 responses for oversized uploads are misleading but not exploitable.
OWASP Category: A05 — Security Misconfiguration (schema completeness) Severity: Informational Status: Open Date Found: 2026-05-18 PR: Story #1478 audit
Description:
All photo route handlers that accept an :id parameter (GET /:id, GET /:id/file, GET /:id/thumbnail, PATCH /:id, DELETE /:id, PUT /:id/annotation, DELETE /:id/annotation) use a shared getPhotoSchema whose params.id is typed as { type: 'string' } with no minLength, maxLength, or pattern constraint. The same gap is present in reorderPhotosSchema items and listPhotosSchema.entityId.
Path traversal is not exploitable here because:
- HTTP path normalization resolves
..segments before they reach the Fastify router. - URL-percent-encoded sequences such as
..%2Fare passed literally topath.join, which does not decode them — resulting in a directory name that contains literal%2Fcharacters (no traversal). - Most critically,
getPhoto(db, id)performs a parameterized SQL query (WHERE id = ?) before any file I/O. A crafted string that is not a UUID will find no row and throwNotFoundError(404) beforepath.joinis ever called.
The gap is purely a schema-completeness / defence-in-depth issue. The pattern used elsewhere in the codebase (e.g., entityId: { minLength: 1, maxLength: 36 } in listPhotosSchema) should be applied consistently to params.id.
Affected Files:
-
server/src/routes/photos.ts:86-94—getPhotoSchemaparams, no minLength/maxLength/pattern onid
Remediation:
Add a UUID format constraint (AJV supports format: 'uuid' via ajv-formats) or a manual pattern:
const getPhotoSchema = {
params: {
type: 'object',
required: ['id'],
properties: {
id: {
type: 'string',
minLength: 36,
maxLength: 36,
pattern: '^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$',
},
},
},
};Risk if Unaddressed: No exploitable vulnerability given the DB-lookup guard. Informational defence-in-depth recommendation only.
OWASP Category: A04 — Insecure Design Severity: Informational Status: Open Date Found: 2026-05-21
Description:
The assignToHouseholdItem path in budgetLineAssignService.ts wraps its four-step mutation in db.transaction(). The assignToWorkItem path performs its orphan check, db.update, and db.select outside a transaction. In theory, a concurrent writer could assign the same line between the check and the update.
Affected Files:
-
server/src/services/budgetLineAssignService.ts:66—assignToWorkItemfunction
Risk if Unaddressed:
Negligible in practice. better-sqlite3 is synchronous and Node.js runs a single event loop thread — no concurrent handler can interleave within the synchronous call chain. The gap is a code consistency issue only.
Remediation:
Wrap assignToWorkItem in db.transaction(() => { ... }) to match the assignToHouseholdItem pattern and make atomicity intent explicit.
OWASP Category: A01 — Broken Access Control (data integrity) Severity: Informational Status: Open Date Found: 2026-05-21
Description:
budgetSourceService.ts:computeUsedAmount sums planned_amount FROM work_item_budgets WHERE budget_source_id = ${sourceId} without filtering work_item_id IS NOT NULL. If Story #1547 creates orphan lines with a non-null budgetSourceId, the budget source usedAmount will be inflated and availableAmount understated.
Affected Files:
-
server/src/services/budgetSourceService.ts:117— missingAND work_item_id IS NOT NULL
Proof of Concept:
Insert an orphan work_item_budgets row (workItemId=null) with budgetSourceId set to an active source. Call GET /api/budget-sources/:id and observe usedAmount includes the orphan's plannedAmount.
Remediation:
Add AND work_item_id IS NOT NULL to the query, consistent with budgetOverviewService line 69 and budgetSourceService line-list query at line 1002:
SELECT planned_amount FROM work_item_budgets
WHERE budget_source_id = ${sourceId} AND work_item_id IS NOT NULLRisk if Unaddressed: Budget source balance display will be incorrect once auto-itemize (Story #1547) starts creating orphan lines with non-null budget source references.
OWASP Category: A03 — Injection (input validation gap) Severity: Informational Status: Open Date Found: 2026-05-21
Description:
The AJV schema in budgetLineAssign.ts declares targetId: { type: 'string' } and the params id: { type: 'string' } without minLength: 1. An empty-string value passes schema validation and reaches the Drizzle query, which returns zero rows and triggers a 404. No exploit path exists — the DB lookup is an implicit guard — but the pattern is inconsistent with stricter schemas elsewhere in the codebase.
Affected Files:
-
server/src/routes/budgetLineAssign.ts:13—targetIdschema -
server/src/routes/budgetLineAssign.ts:22— paramsidschema
Remediation:
Add minLength: 1 to both fields:
targetId: { type: 'string', minLength: 1 },
// params:
id: { type: 'string', minLength: 1 },Risk if Unaddressed: No exploitable vulnerability given the DB-lookup guard. Informational defence-in-depth recommendation only.
OWASP Category: A05 — Security Misconfiguration Severity: Informational Status: Open Date Found: 2026-05-21
Description:
When LLM_BASE_URL is set to a syntactically invalid URL or a URL with a forbidden scheme, the config validation error includes the verbatim value: "LLM_BASE_URL must be a valid URL, got: <value>". This is consistent with how PAPERLESS_URL, PAPERLESS_EXTERNAL_URL, and EXTERNAL_URL are handled throughout the existing codebase. The error is thrown at startup (not request time) and is visible only in server logs / container stderr — not in any HTTP response. Impact is low given the intended audience (the operator who set the env var), but the value could appear in crash-report aggregators or log-shipping infrastructure.
Affected Files:
-
server/src/plugins/config.ts:275—errors.push(\LLM_BASE_URL must be a valid URL, got: ${llmBaseUrl}`)`
Proof of Concept:
Set LLM_BASE_URL=file:///etc/passwd and start the container; the server log at startup will contain that string.
Remediation: Omit the raw URL value from the error message, or truncate/hash it. Example:
errors.push('LLM_BASE_URL must be a valid URL (check the value you configured)');This is a consistent change that should also be applied to the other URL validation error messages in config.ts if log shipping is a concern.
Risk if Unaddressed:
A misconfigured LLM_BASE_URL containing a secret or sensitive path would appear in application logs. No HTTP exposure path exists. Defence-in-depth recommendation only.
OWASP Category: A04 — Insecure Design Severity: Informational Status: Open Date Found: 2026-05-21
Description:
LLM_REQUEST_TIMEOUT_MS validation rejects zero and negative values but has no maximum. A value such as 9999999999 (≈ 115 days) would be accepted as a valid positive integer, effectively making requests hang indefinitely from a practical perspective if the LLM gateway stops responding mid-stream. The AbortController fires correctly for genuine network-level stalls, but an extremely high timeout combined with a provider that streams responses slowly could keep server-side resources (memory, open sockets) tied up far longer than intended.
Affected Files:
-
server/src/plugins/config.ts:279-285—LLM_REQUEST_TIMEOUT_MSvalidation
Remediation: Add a reasonable upper bound, e.g., 5 minutes (300 000 ms):
const MAX_LLM_TIMEOUT_MS = 300_000;
if (isNaN(llmRequestTimeoutMs) || llmRequestTimeoutMs <= 0 || llmRequestTimeoutMs > MAX_LLM_TIMEOUT_MS) {
errors.push(
`LLM_REQUEST_TIMEOUT_MS must be a positive integer ≤ ${MAX_LLM_TIMEOUT_MS}, got: ${llmRequestTimeoutMsStr}`,
);
}Risk if Unaddressed: Accidental misconfiguration could cause long-running LLM requests to consume Node.js async resources for an unreasonable duration. Not exploitable by users. No functional impact on well-operated deployments. Informational only.
OWASP Category: A10 — Server-Side Request Forgery (SSRF) Severity: Informational Status: Open Date Found: 2026-05-21
Description:
The SSRF guard in config.ts rejects non-HTTP/HTTPS schemes (file://, ftp://, etc.) but intentionally allows http://localhost:... and any private-range IP target. This is a deliberate product decision to support self-hosted Ollama deployments (documented in ADR-031 and confirmed in the test suite: LLM_BASE_URL: 'http://localhost:11434/v1' is expected to succeed).
Because LLM_BASE_URL is an operator-controlled environment variable (not user-supplied input), the SSRF threat model differs from a URL taken from an API request. An operator who can set env vars already has equivalent access to internal services via other means.
Affected Files:
-
server/src/plugins/config.ts:264-277— URL scheme validation only, no host allowlist
Remediation: No immediate action required given the operator-trust model. If a future story surfaces this service to user-supplied input, add a host allowlist or private-range block. Document the trade-off in ADR-031 (already partially addressed in the ADR's "Security & Compliance" section).
Risk if Unaddressed:
An operator with malicious intent could point LLM_BASE_URL at an internal metadata endpoint (e.g., http://169.254.169.254). In a single-tenant self-hosted deployment this is not a realistic threat — the operator is the user. Informational tracking only.
OWASP Category: A04 — Insecure Design Severity: Informational Status: Open Date Found: 2026-05-21
Description:
buildUserPrompt(ocrText, hints) concatenates the raw OCR text into the request body without any server-side length limit. For the current Story #1546 implementation this is a service-layer concern only (no HTTP route calls extract() yet), but when Story #1547 wires the route, an unusually large OCR document could generate a request body that:
- Exceeds the context window of the configured LLM model, causing a 400 from the provider.
- Drives up token costs significantly for cloud-hosted providers (OpenAI, Anthropic, etc.).
- Adds latency proportional to OCR size.
The invoiceDate, vendorName, and locale hint fields currently come from server-side DB lookups (not from client request bodies), so they are safe from direct client injection. The ocrText itself also comes from Paperless-ngx OCR content, which is operator-data. No client-supplied prompt injection path exists in this PR.
Affected Files:
-
server/src/services/budgetExtraction/prompts.ts:37-55— no truncation ofocrTextbefore embedding -
server/src/services/budgetExtraction/openAICompatibleProvider.ts:168—buildUserPrompt()result goes directly intomessages[1].content
Remediation:
When Story #1547 implements the auto-itemize route, add a maximum OCR length guard in the service layer before calling extract(). A reasonable cap is 32 000 characters (covering ~20-page German invoices). Truncation with a warning is preferable to rejection:
const MAX_OCR_LENGTH = 32_000;
const safeOcrText = ocrText.length > MAX_OCR_LENGTH
? ocrText.slice(0, MAX_OCR_LENGTH) + '\n[OCR TRUNCATED]'
: ocrText;Risk if Unaddressed: No security risk in this PR (no route exposed). Latency and cost concern for Story #1547. Confirmed not implemented in PR #1550 — implementing agent should address before merge to main.
OWASP Category: A05 — Security Misconfiguration (missing input bounds) Severity: Informational Status: Open Date Found: 2026-05-22
Description:
The AJV schema for POST /api/invoices/:invoiceId/auto-itemize defines lines as a type: 'array' with no maxItems constraint. In commit mode (dryRun: false), the service iterates over every element in the array and inserts one work_item_budgets row and one invoice_budget_lines row per item inside a single transaction. An authenticated operator could submit a request with hundreds or thousands of entries, causing a proportionally large DB insert loop.
In practice the UI only sends lines that originated from a prior LLM dry-run response, and the single-tenant deployment model limits who can call this endpoint. The risk is bounded to operator-level self-DoS.
Affected Files:
-
server/src/routes/invoiceAutoItemize.ts:21—linesschema declarestype: 'array'with nomaxItems
Remediation: Add a reasonable upper bound:
lines: {
type: 'array',
maxItems: 200, // typical invoice has < 50 line items
items: { ... },
},Risk if Unaddressed: No remote attacker vector (requires authentication). Operator-level self-DoS only in a single-tenant model.
OWASP Category: A04 — Insecure Design (missing input bounds) Severity: Low Status: Open Date Found: 2026-05-22
Description:
The JSON schemas for PATCH /api/work-items/:workItemId/budgets/:budgetId and PATCH /api/household-items/:householdItemId/budgets/:budgetId define the move fields as { type: ['string', 'null'] } with no minLength: 1 constraint. An empty string "" passes schema validation. Because the service checks !== null (not truthiness), an empty string is treated as a non-null move target, causing the service to attempt a database lookup for entity ID "". The service correctly throws NotFoundError in that case, so there is no exploit path, but the defence is at the wrong layer. The same pattern exists in the IBL PATCH schema.
Affected Files:
-
server/src/routes/workItemBudgets.ts:80-81—newWorkItemId/newHouseholdItemIdmissingminLength: 1 -
server/src/routes/householdItemBudgets.ts:83-84— same -
server/src/routes/invoiceBudgetLines.ts:57-68— same for the IBL PATCH schema
Remediation:
Add minLength: 1 to the string branch of all move fields:
"newWorkItemId": { "type": ["string", "null"], "minLength": 1 },
"newHouseholdItemId": { "type": ["string", "null"], "minLength": 1 }Risk if Unaddressed: No exploit path — service-layer NotFoundError is the backstop. Unnecessary DB roundtrip on empty-string input.
OWASP Category: A05 — Security Misconfiguration (missing input bounds) Severity: Informational Status: Open Date Found: 2026-05-22
Description:
The schemas for PATCH /api/work-items/:workItemId/budgets/:budgetId and PATCH /api/household-items/:householdItemId/budgets/:budgetId do not include minProperties: 1. An empty body {} passes schema validation and causes the service to run a no-op update (setting only updatedAt). This is a pre-existing absence extended in PR #1554 when the move fields were added to the schemas. The IBL PATCH schema correctly enforces minProperties: 1.
Affected Files:
-
server/src/routes/workItemBudgets.ts:60-93— body schema has nominProperties: 1 -
server/src/routes/householdItemBudgets.ts:65-96— same
Remediation:
Add minProperties: 1 to both schemas, matching the IBL PATCH schema pattern.
Risk if Unaddressed:
No data loss or corruption. Spurious no-op updates with a stale updatedAt touch. No security exploit.
No separate design-phase findings; security was reviewed inline with each implementation PR.
| Date | Scope | Reviewer |
|---|---|---|
| 2026-02-16 | EPIC-01 Auth (PRs #55–#82) | security-engineer |
| 2026-02-17 | EPIC-03 Work Items (PRs #97–#106) | security-engineer |
| 2026-02-20 | EPIC-05 Budget (PRs #150–#158) | security-engineer |
| 2026-02-21 | EPIC-05 Budget Rework (PR #187) | security-engineer |
| 2026-02-22 | EPIC-05 Budget Frontend Rework (PR #193) | security-engineer |
| 2026-02-22 | Budget Hero Bar + Category Filter (PR #195) | security-engineer |
| 2026-02-23 | Standalone Invoices View (PR #203) | security-engineer |
| 2026-02-24 | EPIC-06 Milestones Backend (PR #247) | security-engineer |
| 2026-02-24 | EPIC-06 Scheduling Engine — CPM (PR #248) | security-engineer |
| 2026-02-24 | EPIC-06 Timeline Data API (PR #249) | security-engineer |
| 2026-02-24 | EPIC-06 Gantt Chart Core (PR #250) | security-engineer |
| 2026-02-24 | EPIC-06 Gantt Interactive Features (PR #253) | security-engineer |
| 2026-02-24 | EPIC-06 Milestones Frontend — CRUD Panel & Diamond Markers (PR #254) | security-engineer |
| 2026-02-25 | EPIC-06 UAT Fixes — projected dates, late milestones, WorkItemSelector (PR #263) | security-engineer |
| 2026-02-25 | EPIC-06 UAT Feedback Fixes — column zoom, milestone rows, back-to-timeline nav (PR #267) | security-engineer |
| 2026-02-26 | Gantt dependency highlighting on hover — frontend-only (PR #306) | security-engineer |
| 2026-02-26 | EPIC-07 Actual dates, delay tracking, blocked-status removal (PR #308) | security-engineer |
| 2026-02-27 | Retrospective improvements — dep pinning, shared CSS, formatDate, invoiceService refactor (PR #316) | security-engineer |
| 2026-03-01 | EPIC-08 Paperless-ngx proxy service — backend foundation (PR #362) | security-engineer |
| 2026-03-01 | EPIC-08 PR #362 round 2 — remediation verification (SSRF, content-type allowlist, error sanitization, tags pattern) | security-engineer |
| 2026-03-02 | EPIC-08 Story 8.2 — Document Links Schema & CRUD API (PR #363) | security-engineer |
| 2026-03-02 | EPIC-08 Story 8.3 — Document Browser & Search UI (PR #364) | security-engineer |
| 2026-03-03 | EPIC-04 Stories 4.1–4.7 — Household Items (PRs #396, #397, #398, #399, #400, #401, #402) | security-engineer |
| 2026-03-09 | EPIC-09 Story #470 — User Preferences Infrastructure (PR #708) | security-engineer |
| 2026-03-13 | EPIC-17 Story #747 — CalDAV/CardDAV Feed Endpoints (PR #783) | security-engineer |
| 2026-03-17 | EPIC-17 Story #933 — CalDAV/CardDAV server with DAV token auth and vendor contacts (PR #936) | security-engineer |
| 2026-03-22 | EPIC-19 Story #1146 — Backup & Restore (PR #1150) | security-engineer |
| 2026-05-10 | Issue #1403 — Invoice Deposits: Schema, CRUD API, Cascade (PR #1406) | security-engineer |
| 2026-05-18 | EPIC-16 Story #1478 — Photo Annotator Polish (annotation endpoint security audit) | security-engineer |
| 2026-05-21 | EPIC-20 Story #1545 — Unassigned Budget Lines & One-Shot Parent Assignment (PR #1548) | security-engineer |
| 2026-05-21 | Story #1546 — BudgetExtractionService with OpenAI-compatible LLM gateway (PR #1549) — first outbound LLM integration | security-engineer |
| 2026-05-22 | Story #1547 — Invoice Auto-Itemize POST endpoint and commit service (PR #1550) | security-engineer |
| 2026-05-22 | Story #1553 — Full edit + linked-item move for invoice budget lines (PR #1554) | security-engineer |