feat(invoices): standalone invoices view in budget section

[steilerDev]

Summary

• Adds vendorName field to the Invoice shared type and all invoice API responses (existing vendor-scoped endpoints automatically include it via updated toInvoice() helper)
• New GET /api/invoices endpoint — paginated, filterable by status/vendor/search, sortable, with global status summary (pending/paid/claimed counts + totals)
• New GET /api/invoices/:id endpoint — cross-vendor single invoice lookup
• New Invoices tab in BudgetSubNav between Vendors and Sources
• New /budget/invoices page — summary cards, filter bar, sortable table (desktop) + card list (mobile), pagination, create invoice modal with vendor dropdown
• New /budget/invoices/:id detail page — full invoice details, edit modal, delete with navigation back to list

What's Changed

Shared (shared/src/)

• types/invoice.ts: vendorName: string added to Invoice; new InvoiceStatusSummary, InvoiceSummary, InvoiceListPaginatedResponse, InvoiceDetailResponse types
• index.ts: exports for new types

Server (server/src/)

• services/invoiceService.ts: toInvoice() now resolves vendor name; listAllInvoices() + getInvoiceById() added
• routes/standaloneInvoices.ts (new): GET / and GET /:invoiceId
• app.ts: registers standalone invoice routes at /api/invoices

Client (client/src/)

• lib/invoicesApi.ts: fetchAllInvoices() + fetchInvoiceById() added
• components/BudgetSubNav/BudgetSubNav.tsx: Invoices tab added
• pages/InvoicesPage/ (new): list page with summary cards + filters + pagination + create modal
• pages/InvoiceDetailPage/ (new): detail page with edit/delete modals
• App.tsx: two new routes registered

Test plan

• /budget/invoices — "Invoices" tab is active, summary cards show correct counts/totals, table renders with sortable columns, filters (search/status/vendor) narrow results
• Clicking an invoice row's View link navigates to /budget/invoices/:id
• /budget/invoices/:id — invoice details render correctly, vendor name is a link to the vendor page, Edit modal saves changes, Delete navigates back to list
• "Add Invoice" modal requires vendor selection; creates invoice and refreshes list
• Existing vendor detail page still shows invoices correctly (vendorName is present in responses but not displayed there)
• All CI checks pass

🤖 Generated with Claude Code

[steilerDev]

[security-engineer]

Security Review — PR #203: Standalone Invoices View

Reviewed: server/src/routes/standaloneInvoices.ts, server/src/services/invoiceService.ts, client/src/pages/InvoicesPage/InvoicesPage.tsx, client/src/pages/InvoiceDetailPage/InvoiceDetailPage.tsx, client/src/lib/invoicesApi.ts, shared/src/types/invoice.ts

---

Checklist

• No SQL/command/XSS injection vectors in new code
• Authentication/authorization enforced on all new endpoints
• No sensitive data (secrets, tokens, PII) exposed in logs, errors, or client responses
• User input validated and sanitized at API boundaries
• New dependencies — none added
• No hardcoded credentials or secrets
• CORS configuration remains restrictive
• Error responses do not leak internal details

---

Findings

Authentication and Authorization — PASS

Both new endpoints (GET /api/invoices and GET /api/invoices/:invoiceId) correctly enforce authentication via two layers:

1. The global preValidation hook in auth.ts blocks all /api/* routes not in PUBLIC_ROUTES. The new routes are not public, so unauthenticated requests are rejected with 401 before reaching the handler.
2. Each handler also contains an explicit if (!request.user) throw new UnauthorizedError() guard as a belt-and-suspenders check.

This is consistent with the pattern used across all other protected routes in the codebase.

Input Validation — PASS

GET /api/invoices querystring schema is well-formed:

• page/pageSize: integer with min/max bounds
• status/sortBy/sortOrder: enum-constrained — only allowlisted values are accepted
• q: string with maxLength: 200
• additionalProperties: false prevents unexpected query parameters from reaching handler code

SQL Injection — PASS

The sortBy parameter is enum-validated by AJV before reaching service code, and then mapped to a Drizzle ORM column reference (not a raw string). The LIKE search for q correctly escapes % and _ metacharacters before constructing the pattern, and uses a parameterized Drizzle sql tagged template. No raw SQL string interpolation exists anywhere in the new code.

XSS — PASS

All user-supplied data is rendered through React JSX expressions, which auto-escape output. No dangerouslySetInnerHTML, innerHTML, or eval() usage was found in either new page. The ApiClientError.error.message values reflected in error banners originate from fixed strings in AppError subclasses on the server — they never echo user input back, so there is no reflected XSS path.

Information Disclosure — PASS (by design)

GET /api/invoices returns invoices across all vendors. For this single-tenant self-hosted application, all authenticated users have access to the same project data — this is by design and consistent with how other listing endpoints work (GET /api/work-items, GET /api/vendors, etc.).

Regression on Previous Findings — RESOLVED

The prior finding "pageSize 200 Exceeds Server Maximum of 100" (PR #193) is resolved in this PR. Both InvoicesPage.tsx and InvoiceDetailPage.tsx correctly use pageSize: 100 for work item fetching. The .catch() handlers for fetchWorkItemBudgets are also properly implemented in both pages, addressing the related "Swallowed Promise Rejection" pattern identified in PR #193.

---

Informational Finding

getInvoiceByIdSchema is missing additionalProperties: false on the params schema
(server/src/routes/standaloneInvoices.ts, lines ~3317-3325)

This is a style inconsistency — the listAllInvoicesSchema querystring in the same file correctly uses additionalProperties: false, but the params schema for the :invoiceId route does not. This has no exploitable impact (path parameters are positionally extracted and cannot carry extra fields), but it is inconsistent with the defensive validation style used elsewhere in the codebase. Recommended fix:

const getInvoiceByIdSchema = {
  params: {
    type: 'object',
    required: ['invoiceId'],
    properties: {
      invoiceId: { type: 'string' },
    },
    additionalProperties: false,  // add this
  },
};

This can be addressed in a future PR — it does not block merge.

---

Verdict

No blocking security issues found. The implementation correctly enforces authentication, validates all querystring inputs against strict enums and type/range constraints, uses parameterized queries throughout (no SQL injection), and renders all output via React's auto-escaping (no XSS). One informational finding (params schema completeness) noted for a future cleanup pass.

Security Audit wiki page updated: https://github.com/steilerDev/cornerstone/wiki/Security-Audit

[steilerDev]

[product-architect] Architecture review of PR #203 — standalone invoices view.

Overall Assessment

The PR adds two new backend endpoints (GET /api/invoices, GET /api/invoices/:invoiceId), corresponding API client functions, and two new frontend pages. The implementation is well-structured and follows most established patterns. However, there are several issues that need to be addressed before merge — one breaking change to the shared type, a wiki deviation, missing test coverage, and an N+1 query regression on existing endpoints.

Verdict: Request Changes (4 items must be addressed)

Issues Requiring Changes

1. CRITICAL — Wiki API Contract deviation (new endpoints + modified Invoice shape not documented)

The wiki API Contract (wiki/API-Contract.md) does not document:

• GET /api/invoices (paginated cross-vendor listing)
• GET /api/invoices/:invoiceId (cross-vendor single invoice)
• The vendorName field added to the Invoice response shape

Per the Implementation-Wiki Deviation Workflow in CLAUDE.md: "Fix and wiki update should land together — do not merge code that knowingly contradicts the wiki without also updating the wiki in the same PR."

The Invoice response shape in the wiki (line ~2504) currently shows:

interface InvoiceResponse {
  id: string;
  vendorId: string;
  // ... no vendorName
}

Action required: Update wiki/API-Contract.md to:

1. Add vendorName: string to the Invoice Response Shape section
2. Document GET /api/invoices with query parameters, response shape (including InvoiceListPaginatedResponse), and error codes
3. Document GET /api/invoices/:invoiceId with path parameters, response shape (InvoiceDetailResponse), and error codes
4. Add a Deviation Log entry noting the vendorName field was added to the Invoice shape
5. Commit the wiki submodule update alongside the code changes

2. N+1 vendor query regression on existing endpoints

In server/src/services/invoiceService.ts, the toInvoice() helper (line 51-72) now executes a vendor lookup query per row:

function toInvoice(db: DbType, row: typeof invoices.$inferSelect): Invoice {
  const vendor = db.select().from(vendors).where(eq(vendors.id, row.vendorId)).get();
  // ...
}

This function is called by:

• listInvoices() (line 98) — the vendor-scoped list at GET /api/vendors/:vendorId/invoices. This already knows the vendor from the URL path, so the per-row vendor query is unnecessary. For N invoices, this adds N redundant queries.
• createInvoice() (line 306) — acceptable (single row)
• updateInvoice() (line 398) — acceptable (single row)
• getInvoiceById() (line 236) — acceptable (single row)

Meanwhile, listAllInvoices() correctly uses a JOIN to resolve vendor names efficiently, then bypasses toInvoice() with its own mapping logic (lines 199-217). This is good but results in duplicated mapping code.

Action required: Refactor toInvoice() to accept an optional vendorName parameter so callers that already know the vendor name can pass it in, avoiding the redundant query. For example:

function toInvoice(db: DbType, row: typeof invoices.$inferSelect, vendorName?: string): Invoice {
  const resolvedVendorName = vendorName 
    ?? db.select().from(vendors).where(eq(vendors.id, row.vendorId)).get()?.name 
    ?? 'Unknown';
  // ...
}

Then listInvoices() can pass the known vendor name, and listAllInvoices() can reuse toInvoice() instead of duplicating the mapping.

3. Missing test coverage for new backend service functions and API client functions

No unit tests exist for the new service functions listAllInvoices() and getInvoiceById() in invoiceService.ts. No tests exist for the new client API functions fetchAllInvoices() and fetchInvoiceById() in invoicesApi.ts.

The existing invoicesApi.test.ts only covers fetchInvoices, createInvoice, updateInvoice, and deleteInvoice. The vendorName field was added to test fixtures (good), but no new test cases were added for the two new functions.

Action required: The QA agent needs to add:

• Unit tests for invoiceService.listAllInvoices() covering: pagination, search filter, status filter, vendor filter, sorting, summary computation, empty results
• Unit tests for invoiceService.getInvoiceById() covering: found, not found (404)
• Client API tests for fetchAllInvoices() covering: URL construction with various query param combinations, response parsing
• Client API tests for fetchInvoiceById() covering: URL construction, envelope unwrapping (.then(r => r.invoice))

4. InvoiceDetailResponse type is defined but not used in the route handler

In shared/src/types/invoice.ts, InvoiceDetailResponse is defined and exported:

export interface InvoiceDetailResponse {
  invoice: Invoice;
}

But in server/src/routes/standaloneInvoices.ts line 63, the response is constructed inline:

return reply.status(200).send({ invoice });

This is not a blocking issue — it works correctly — but the type should be referenced in the API client for type safety. Currently fetchInvoiceById() uses get<{ invoice: Invoice }> instead of get<InvoiceDetailResponse>. Consider using the shared type for consistency.

Minor Observations (Non-blocking)

5. Duplicate formatCurrency and formatDate utility functions

Both InvoicesPage.tsx and InvoiceDetailPage.tsx define identical formatCurrency() and formatDate() helper functions. The VendorDetailPage likely has similar helpers. Consider extracting these into a shared client/src/lib/format.ts utility in a future PR to reduce duplication. Not blocking for this PR.

6. CSS duplication across page modules

The two new CSS modules (1,642 lines total) contain substantial duplication of button styles, modal styles, form styles, etc. that are repeated across VendorDetailPage, InvoicesPage, and InvoiceDetailPage. This is consistent with the existing pattern (each page is self-contained with CSS Modules), but the growing duplication suggests a future PR should extract shared component CSS. Not blocking for this PR.

7. fetchAllInvoices unwrapping vs fetchInvoiceById unwrapping

fetchAllInvoices() returns the full InvoiceListPaginatedResponse (caller accesses .invoices, .pagination, .summary), while fetchInvoiceById() unwraps the envelope in the API client:

return get<{ invoice: Invoice }>(`/invoices/${invoiceId}`).then((r) => r.invoice);

The existing pattern in invoicesApi.ts is mixed — fetchInvoices() unwraps, createInvoice() unwraps, but fetchAllInvoices() does not. This is fine for this PR since the list response has multiple top-level keys. Just noting the inconsistency for awareness.

What Looks Good

• Routing: Lazy loading of both new pages in App.tsx follows the established pattern correctly
• Route nesting: /budget/invoices and /budget/invoices/:id are correctly nested under the budget route group
• BudgetSubNav: Tab placement between Vendors and Sources is logical
• Shared types: InvoiceStatusSummary, InvoiceSummary, InvoiceListPaginatedResponse, InvoiceDetailResponse are well-defined and properly exported
• Backend route structure: standaloneInvoices.ts follows the same pattern as other route files (Fastify schema validation, auth check, service delegation)
• Fastify schema validation: Correct use of JSON Schema for query parameter validation with additionalProperties: false
• Service layer: listAllInvoices() correctly uses JOINs for the paginated listing, properly handles WHERE clause composition, and computes an unfiltered summary
• CSS Modules: Proper use of design tokens throughout, responsive breakpoints for mobile/tablet, touch-friendly minimum heights
• Frontend patterns: URL-based filter state via useSearchParams, debounced search, proper loading/error states, accessible modals with role="dialog" and aria-modal
• Test fixture updates: Existing test fixtures in invoicesApi.test.ts and VendorDetailPage.test.tsx correctly updated with vendorName field

Summary of Required Changes

1. Update wiki API Contract with new endpoints and vendorName field — must land with this PR
2. Fix N+1 vendor query in toInvoice() for the existing listInvoices() path
3. Add missing unit tests for new service functions and API client functions (QA agent)
4. Consider using InvoiceDetailResponse in fetchInvoiceById for type consistency (minor)

[github-actions]

🎉 This PR is included in version 1.9.0-beta.46 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀