Repository navigation
Fix/security hardening uploads - #10
Conversation
- Add global secureHeaders middleware for baseline API security - Fix magic byte validation truncation and enforce verified MIME types - Add Zod schema validation and UUID-based collision-resistant R2 keys - Fix recalculation cascade loop boundary and add structural JSON checks - Add authorization boundary tests (cross-user notifications, inactive tenancies) - Update CONTRIBUTING.md pre-push hook documentation.
…ection Implements end-of-file consistency checks in the image upload pipeline to detect and block polyglot payloads (e.g. ZIP/HTML data appended after a valid image). Changes include: - JPEG: Scan backwards for the last End-of-Image (FF D9) marker while safely ignoring FF 00 byte-stuffing - PNG: Walk chunks sequentially and enforce that no bytes exist after the IEND chunk - WebP: Validate the RIFF container's declared ChunkSize against the actual file buffer length - Tests: Add 12+ regression tests for appended payloads, truncated files, and structurally clean images to prevent false positives - Update legacy test mocks to use structurally sound image stubs.
📝 WalkthroughWalkthroughThe PR strengthens image validation, updates upload storage and download handling, adds security headers, tightens API validation and ownership checks, expands tests, and updates project guidance and tooling. ChangesFile validation and upload security
API validation and authorization
Project guidance and tooling
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/api/routes/uploads.ts (1)
143-166: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReport validation failures distinctly from missing fields.
The branch at Line 152 handles two different causes. A missing
photoand a schema failure both returnMISSING_FIELDSwith the text "photo, periodId, propertyId, and purpose are required". If a client sends an unsupportedpurposevalue, the response text does not describe the real cause. Return the parse issue for schema failures.🐛 Proposed fix to distinguish the two causes
if (!photo || !fieldsParsed.success) { await db.run( sql`UPDATE upload_daily_count SET count = count - 1 WHERE id = ${counterId}` ); return c.json( { success: false as const, error: { - code: "MISSING_FIELDS", - message: "photo, periodId, propertyId, and purpose are required", + code: !photo ? "MISSING_FIELDS" : "INVALID_FIELDS", + message: !photo + ? "photo, periodId, propertyId, and purpose are required" + : `Invalid upload fields: ${fieldsParsed.error.issues + .map((i) => i.path.join(".")) + .join(", ")}`, }, }, 400 ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/api/routes/uploads.ts` around lines 143 - 166, Split the combined guard around photoEntry and fieldsParsed into distinct branches: keep missing photo handling as MISSING_FIELDS, but when uploadFormSchema.safeParse fails, return a validation response using fieldsParsed.error issues instead of the missing-fields message. Ensure both paths still decrement the daily upload counter before returning.
🧹 Nitpick comments (3)
src/api/routes/uploads.test.ts (1)
70-74: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
vi.mockedhere for consistency and type safety.Line 327 already uses
vi.mocked(validateUploadedFile). The cast toReturnType<typeof vi.fn>at Line 70 discards the argument and return types, so a resolved value that does not match the validator contract would not fail type checking.♻️ Proposed change
- (validateUploadedFile as ReturnType<typeof vi.fn>).mockResolvedValue({ + vi.mocked(validateUploadedFile).mockResolvedValue({ valid: true, verifiedMimeType: "image/jpeg", buffer: mockBuffer, });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/api/routes/uploads.test.ts` around lines 70 - 74, Replace the ReturnType<typeof vi.fn> cast in the validateUploadedFile mock setup with vi.mocked(validateUploadedFile), matching the existing usage elsewhere in the test and preserving the validator’s argument and return type checking.src/api/routes/uploads.ts (1)
236-237: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winFail closed when the validator returns no buffer.
validateUploadedFilealways returnsbuffertogether withvalid: true. The?? (await photo.arrayBuffer())fallback therefore never runs today. If a future change makesbufferoptional on the success path, this line silently stores bytes that were never structurally validated. Treat a missing buffer as an error instead.♻️ Proposed fail-closed handling
- await r2.put(objectKey, validation.buffer ?? (await photo.arrayBuffer()), { + if (!validation.buffer) { + throw new Error("File validation returned no buffer"); + } + await r2.put(objectKey, validation.buffer, { httpMetadata: { contentType: verifiedMime },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/api/routes/uploads.ts` around lines 236 - 237, Update the upload flow around validateUploadedFile and the r2.put call to reject a successful validation result when buffer is missing, returning an error instead of reading photo.arrayBuffer(). Pass only the validated buffer to r2.put and preserve the existing behavior for valid results that include it.src/api/lib/file-validation.ts (1)
120-132: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winRequire a
0xFFprefix before each JPEG marker byte.The outer loop reads
bytes[offset]as a marker byte even when no0xFFprecedes it. The inner fill-byte loop at Line 122 only runs when the current byte is already0xFF. If the current byte is any other value, the walk treats that byte as a marker and continues. This accepts malformed marker chains that a strict parser rejects.The EOI-at-EOF rule still blocks appended payloads, so this is hardening, not an exploitable bypass.
♻️ Proposed strict marker-prefix check
let offset = 2; // skip SOI while (offset < len) { + // A marker must start with at least one 0xFF byte. + if (bytes[offset] !== 0xff) { + return { + ok: false, + error: "JPEG marker is missing its FF prefix — corrupted file.", + }; + } // Skip 0xFF fill bytes preceding the marker byte while (offset < len && bytes[offset] === 0xff) { offset++; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/api/lib/file-validation.ts` around lines 120 - 132, Update the marker-processing loop around the markerByte read to require bytes[offset] to be 0xFF before consuming a marker byte; otherwise return the existing malformed-JPEG validation error. Preserve the current fill-byte skipping and EOI-at-EOF handling while ensuring every accepted marker has the required FF prefix.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CONTRIBUTING.md`:
- Line 90: Align the pre-push command documented in CONTRIBUTING.md and
AGENTS.md with the actual .husky/pre-push behavior: either update all references
and the hook to use the same contract, preferably astro check, or explicitly
document the intentional difference. Ensure no stale command descriptions
remain.
In `@src/api/app.ts`:
- Around line 88-90: Update the secureHeaders configuration to make the complete
security-header policy explicit: either set the unused default options for
Cross-Origin-Resource-Policy, Cross-Origin-Opener-Policy, Referrer-Policy, and
Strict-Transport-Security to false, or explicitly retain those defaults and add
response-header tests. Preserve the existing xFrameOptions and
xContentTypeOptions settings.
In `@src/api/lib/recalculation.ts`:
- Around line 46-70: Preserve the validated oneOffCharges and rateOverride
snapshot through recalculation: update the surrounding validation flow and
recalculateBillsForPeriod call to pass the parsed values into bill deletion and
generation instead of rereading mutable period rows. If the helper cannot accept
snapshots, make validation, deletion, and generation atomic or serialize
concurrent recalculations for the same period.
In `@src/api/routes/uploads.ts`:
- Line 234: In the upload handler, after the property authorization block and
before constructing the object key, load the billing period and require its id
and propertyId to match using the existing billingPeriods query pattern. If no
matching period exists, decrement the daily upload counter and return the
established NOT_FOUND response; only then use periodId in the R2 key and
billPhotos record.
---
Outside diff comments:
In `@src/api/routes/uploads.ts`:
- Around line 143-166: Split the combined guard around photoEntry and
fieldsParsed into distinct branches: keep missing photo handling as
MISSING_FIELDS, but when uploadFormSchema.safeParse fails, return a validation
response using fieldsParsed.error issues instead of the missing-fields message.
Ensure both paths still decrement the daily upload counter before returning.
---
Nitpick comments:
In `@src/api/lib/file-validation.ts`:
- Around line 120-132: Update the marker-processing loop around the markerByte
read to require bytes[offset] to be 0xFF before consuming a marker byte;
otherwise return the existing malformed-JPEG validation error. Preserve the
current fill-byte skipping and EOI-at-EOF handling while ensuring every accepted
marker has the required FF prefix.
In `@src/api/routes/uploads.test.ts`:
- Around line 70-74: Replace the ReturnType<typeof vi.fn> cast in the
validateUploadedFile mock setup with vi.mocked(validateUploadedFile), matching
the existing usage elsewhere in the test and preserving the validator’s argument
and return type checking.
In `@src/api/routes/uploads.ts`:
- Around line 236-237: Update the upload flow around validateUploadedFile and
the r2.put call to reject a successful validation result when buffer is missing,
returning an error instead of reading photo.arrayBuffer(). Pass only the
validated buffer to r2.put and preserve the existing behavior for valid results
that include it.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 33962df9-cad0-4f91-9005-9cfd0387c2e7
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
AGENTS.mdCONTRIBUTING.mdpackage.jsonsrc/api/app.tssrc/api/lib/file-validation.test.tssrc/api/lib/file-validation.tssrc/api/lib/property-auth.test.tssrc/api/lib/recalculation.tssrc/api/routes/custom-charges.test.tssrc/api/routes/notifications.test.tssrc/api/routes/notifications.tssrc/api/routes/uploads.test.tssrc/api/routes/uploads.tstsconfig.json
💤 Files with no reviewable changes (1)
- AGENTS.md
|
|
||
| - **Pre-commit:** Runs ESLint and Prettier on staged files via `lint-staged`. Fix any lint errors before committing. | ||
| - **Pre-push:** Runs `astro check` (full TypeScript typecheck). Fix any type errors before pushing. | ||
| - **Pre-push:** Runs `pnpm typecheck`, `pnpm lint`, and `pnpm test`. Fix any errors before pushing. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- CONTRIBUTING.md around line 90 ---\n'
sed -n '75,100p' CONTRIBUTING.md || true
printf '\n--- package scripts related to husky/pre-push/typecheck/lint/test/astro ---\n'
if [ -f package.json ]; then
python3 - <<'PY'
import json
from pathlib import Path
p=Path('package.json')
data=json.loads(p.read_text())
print(json.dumps(data.get('scripts', {}), indent=2, sort_keys=True))
PY
fi
printf '\n--- git hooks files ---\n'
if [ -d .husky ]; then
find .husky -maxdepth 2 -type f -print | sort | xargs -r -n1 sh -c 'echo "### $0"; sed -n "1,160p" "$0"'
else
echo "no .husky directory"
fi
printf '\n--- references to astro check and pre-push hooks ---\n'
rg -n "astro check|pre-push|typecheck|lint|test" --glob '!node_modules' --glob '!dist' --glob '!build' . | sed -n '1,220p'Repository: VaibhavDaveDev/Meterly
Length of output: 251
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "--- CONTRIBUTING.md around line 90 ---"
sed -n '75,100p' CONTRIBUTING.md || true
echo
echo "--- package scripts ---"
if [ -f package.json ]; then
python3 - <<'PY'
import json
from pathlib import Path
data=json.loads(Path('package.json').read_text())
print(json.dumps(data.get('scripts', {}), indent=2, sort_keys=True))
PY
fi
echo
echo "--- git hooks files ---"
if [ -d .husky ]; then
find .husky -maxdepth 2 -type f -print | sort | while read -r f; do
echo "### $f"
sed -n '1,160p' "$f"
done
else
echo "no .husky directory"
fi
echo
echo "--- references to pre-push hooks and checks ---"
rg -n "astro check|pre-push|typecheck|lint|test" --glob '!node_modules' --glob '!dist' --glob '!build' . | sed -n '1,220p'Repository: VaibhavDaveDev/Meterly
Length of output: 20023
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "--- AGENTS.md around Husky section ---"
sed -n '120,130p' AGENTS.md || true
echo
echo "--- git status for Husky files ---"
git status --short -- .husky package.json AGENTS.md CONTRIBUTING.md || true
echo
echo "--- diff for .husky/pre-push if present ---"
git diff -- .husky/pre-push package.json AGENTS.md CONTRIBUTING.md | sed -n '1,220p'Repository: VaibhavDaveDev/Meterly
Length of output: 1693
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "--- list tracked and unstaged/untracked git files of interest ---"
git ls-files --error-unmatch .husky/pre-push package.json AGENTS.md CONTRIBUTING.md || true
echo
echo "--- read .husky/pre-push if tracked ---"
if git ls-files --error-unmatch .husky/pre-push >/dev/null 2>&1; then
cat -n .husky/pre-push
fi
echo
echo "--- search all checkout paths for husky pre-push name ---"
find /tmp -maxdepth 3 -path '*husky*pre-push*' -o -path '*pre-push*' 2>/dev/null | sed -n '1,120p' || trueRepository: VaibhavDaveDev/Meterly
Length of output: 606
Keep the pre-push hook and contributor docs aligned with the agent contract.
The current .husky/pre-push still runs pnpm typecheck, pnpm lint, and pnpm test -- run, while AGENTS.md documents the pre-push contract as astro check. Update the hook and docs to match one another, or make the change explicit if this is intentional.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CONTRIBUTING.md` at line 90, Align the pre-push command documented in
CONTRIBUTING.md and AGENTS.md with the actual .husky/pre-push behavior: either
update all references and the hook to use the same contract, preferably astro
check, or explicitly document the intentional difference. Ensure no stale
command descriptions remain.
Source: Coding guidelines
| secureHeaders({ | ||
| xFrameOptions: "SAMEORIGIN", | ||
| xContentTypeOptions: "nosniff", |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 6 \
'secureHeaders|crossOriginResourcePolicy|crossOriginOpenerPolicy|strictTransportSecurity|referrerPolicy|xFrameOptions|xContentTypeOptions' \
src/api/app.ts package.json
rg -n -C 4 \
'cors\(|Access-Control-Allow-Origin|fetch\(|XMLHttpRequest|<img|download|PUBLIC_.*API|VITE_.*API|API_URL' \
src --glob '*.{ts,tsx,astro}'
rg -n -C 3 \
'X-Frame-Options|X-Content-Type-Options|Cross-Origin-Resource-Policy|Strict-Transport-Security' \
src --glob '*.{test,spec}.{ts,tsx}'Repository: VaibhavDaveDev/Meterly
Length of output: 50378
🏁 Script executed:
#!/usr/bin/env bash
set -u
printf '--- src/api/app.ts imports and secureHeaders/cors sections ---\n'
sed -n '1,155p' src/api/app.ts | cat -n
printf '\n--- package hono version ---\n'
node - <<'JS'
const fs = require('fs');
const pkg = JSON.parse(fs.readFileSync('package.json','utf8'));
console.log({
dependencies: pkg.dependencies?.hono,
devDependencies: pkg.devDependencies?.hono
});
JS
printf '\n--- tests referencing response headers ---\n'
rg -n 'headers?|X-Frame-Options|X-Content-Type-Options|Cross-Origin-Resource-Policy|Cross-Origin-Opener-Policy|Strict-Transport-Security|Referrer-Policy' src --glob '*.{test,spec}.{ts,tsx}' || true
printf '\n--- app.ts route/export headers references ---\n'
rg -n 'Download-File|Content-Disposition|Content-Type|Set-Cookie|Security-Policy|Cross-Origin-Opener-Policy|Cross-Origin-Resource-Policy|Strict-Transport-Security|xFrameOptions|crossOriginResourcePolicy|xContentTypeOptions|referrerPolicy|frameOptions' src/api src --glob '*.{ts,tsx}' || trueRepository: VaibhavDaveDev/Meterly
Length of output: 26207
🌐 Web query:
Hono secureHeaders default options Cross-Origin-Resource-Policy Cross-Origin-Opener-Policy Referrer-Policy Strict-Transport-Security
💡 Result:
The Hono secureHeaders middleware applies several security headers by default when called without custom options [1]. Below are the default values for the specific headers requested: - Cross-Origin-Resource-Policy: same-origin [2][1] - Cross-Origin-Opener-Policy: same-origin [1] - Referrer-Policy: no-referrer [1] - Strict-Transport-Security: max-age=15552000; includeSubDomains [2][1] When enabled by default, these headers are included in the response automatically [1]. The middleware also sets other security-related headers such as X-Content-Type-Options: nosniff, X-Frame-Options: SAMEORIGIN, and removes the X-Powered-By header [2][1]. You can override these defaults by passing a configuration object to the secureHeaders() middleware function [1].
Citations:
- 1: https://honojs-hono.mintlify.app/api/middleware/secure-headers
- 2: https://hono.dev/docs/middleware/builtin/secure-headers
Make the effective security-header policy explicit.
secureHeaders still sends defaults such as Cross-Origin-Resource-Policy: same-origin, Cross-Origin-Opener-Policy: same-origin, Referrer-Policy: no-referrer, and Strict-Transport-Security when only xFrameOptions and xContentTypeOptions are provided. Set the unused options to false, or keep the defaults with an explicit option list and add response-header tests.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/api/app.ts` around lines 88 - 90, Update the secureHeaders configuration
to make the complete security-header policy explicit: either set the unused
default options for Cross-Origin-Resource-Policy, Cross-Origin-Opener-Policy,
Referrer-Policy, and Strict-Transport-Security to false, or explicitly retain
those defaults and add response-header tests. Preserve the existing
xFrameOptions and xContentTypeOptions settings.
Source: MCP tools
| // Mirror the execution-loop boundary: stop before a non-target confirmed period. | ||
| // The start period (startPeriodId) always validates even if confirmed, | ||
| // because explicit recalculation is allowed to override confirmed periods. | ||
| if (p.id !== startPeriodId && p.status === "confirmed") break; | ||
|
|
||
| if (p.oneOffCharges) { | ||
| try { | ||
| JSON.parse(p.oneOffCharges); | ||
| const parsed: unknown = JSON.parse(p.oneOffCharges); | ||
| if ( | ||
| !Array.isArray(parsed) || | ||
| parsed.some((charge) => { | ||
| if (typeof charge !== "object" || charge === null) return true; | ||
| const c = charge as Record<string, unknown>; | ||
| return ( | ||
| typeof c.chargedToTenant !== "boolean" || | ||
| typeof c.amount !== "number" || | ||
| !Number.isFinite(c.amount) || | ||
| typeof c.name !== "string" | ||
| ); | ||
| }) | ||
| ) { | ||
| throw new TypeError( | ||
| "oneOffCharges must be an array of {chargedToTenant: boolean, amount: number, name: string}" | ||
| ); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 8 \
'\brecalculateChain\s*\(|\brecalculateBillsForPeriod\s*\(|\boneOffCharges\b|\brateOverride\b|\b(batch|transaction)\b' \
--glob '*.ts' --glob '*.tsx' --glob '*.sql' || trueRepository: VaibhavDaveDev/Meterly
Length of output: 160
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf 'Tracked files matching recalculation or billing:'
git ls-files | grep -E 'recalculation|bill|billing|rate' || true
printf '\nFiles in src/api/lib:'
git ls-files src/api/lib || true
printf '\nSearch all tracked files for relevant symbols (limited):'
rg -n -C 4 \
'recalculateChain|recalculateBillsForPeriod|oneOffCharges|rateOverride|confirm|billing|recalculate|deleted|Bill' \
--glob '*.ts' --glob '*.tsx' --glob '*.sql' --glob '*.astro' \
|| trueRepository: VaibhavDaveDev/Meterly
Length of output: 1997
Preserve one data snapshot through validation and recalculation.
The pre-validation reads oneOffCharges and rateOverride, but recalculateBillsForPeriod reads them again from the rows used for bill deletion. If another request changes either value between the reads, bill deletion can succeed while later bill generation fails or skips values, leaving the period without bills.
Pass the validated parsed values into the helper, or make validation, deletion, and bill generation atomic. If a transaction is not available, callers must serialize concurrent recalculations for the same period.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/api/lib/recalculation.ts` around lines 46 - 70, Preserve the validated
oneOffCharges and rateOverride snapshot through recalculation: update the
surrounding validation flow and recalculateBillsForPeriod call to pass the
parsed values into bill deletion and generation instead of rereading mutable
period rows. If the helper cannot accept snapshots, make validation, deletion,
and generation atomic or serialize concurrent recalculations for the same
period.
| const ext = MIME_TO_EXT[photo.type] ?? "bin"; | ||
| objectKey = `${propertyId}/${periodId}/${user.id}/${timestamp}.${ext}`; | ||
| const ext = MIME_TO_EXT[verifiedMime] ?? "bin"; | ||
| objectKey = `${propertyId}/${periodId}/${user.id}/${timestamp}-${uid}.${ext}`; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Validate periodId before using it in the R2 object key.
uploadFormSchema accepts any non-empty string for periodId, and the handler never checks that the period exists or belongs to propertyId. The value is interpolated directly into the object key. An authorized user can therefore submit a periodId such as x/../other-property/y and write objects under an unrelated key prefix. The same value is stored as billPhotos.billingPeriodId at Line 281, which creates a row that references a non-existent period.
Read authorization stays safe because the download route resolves ownership from the billPhotos row. The impact is key-namespace pollution and orphaned rows, not data disclosure.
Load the billing period and confirm it belongs to propertyId, as the list route already does at Lines 349-353.
🛡️ Proposed period ownership check before storage
const validation = await validateUploadedFile(
photo,
purpose === "bill_document" ? "bill-document" : "meter-photo"
);Add this check after the property authorization block at Line 197:
const [period] = await db
.select()
.from(billingPeriods)
.where(
and(
eq(billingPeriods.id, periodId),
eq(billingPeriods.propertyId, propertyId)
)
)
.limit(1);
if (!period) {
await db.run(
sql`UPDATE upload_daily_count SET count = count - 1 WHERE id = ${counterId}`
);
return c.json(
{
success: false as const,
error: { code: "NOT_FOUND", message: "Billing period not found" },
},
404
);
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/api/routes/uploads.ts` at line 234, In the upload handler, after the
property authorization block and before constructing the object key, load the
billing period and require its id and propertyId to match using the existing
billingPeriods query pattern. If no matching period exists, decrement the daily
upload counter and return the established NOT_FOUND response; only then use
periodId in the R2 key and billPhotos record.
| "strict": true, | ||
| "skipLibCheck": true, | ||
| "lib": ["ESNext", "DOM", "DOM.Iterable"], | ||
| "lib": ["ESNext"], |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n 'HTML[A-Za-z]+Element|window\.|document\.' --glob '*.{ts,tsx,astro}' .
rg -n '"lib"|DOM\.Iterable|DOM' --glob 'tsconfig*.json' .Repository: VaibhavDaveDev/Meterly
Length of output: 18356
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== files =="
git ls-files | sed -n '1,200p'
echo "== tsconfig =="
cat -n tsconfig.json
echo "== package scripts/typescript refs =="
python3 - <<'PY'
import json, pathlib
for p in pathlib.Path('.').glob('package.json'):
data=json.loads(p.read_text())
print('---', p)
print(json.dumps({k:data.get(k) for k in ['type','scripts','dependencies','devDependencies']}, indent=2, sort_keys=True))
PYRepository: VaibhavDaveDev/Meterly
Length of output: 10191
🌐 Web query:
TypeScript compilerOptions lib ESNext includes DOM library default
💡 Result:
In TypeScript, the default set of libraries (lib) is determined by your target setting [1][2]. If you do not explicitly define the lib option in your tsconfig.json, TypeScript automatically injects a default set of library files appropriate for that target [3]. Crucially, when you set lib manually, this default behavior is overridden, and you must explicitly include all necessary library files, including DOM if your project requires browser APIs [4][5]. Regarding the ESNext target: 1. When target is set to ESNext (or any other value), the lib option is not automatically populated with DOM unless it is part of the standard default set for that specific target [1]. 2. Historically, setting target to ESNext did not include DOM by default [4], leading to "cannot find name 'document'" type errors. 3. If you require DOM support while using ESNext, you must explicitly specify it in your lib array [4]: { "compilerOptions": { "target": "ESNext", "lib": ["ESNext", "DOM", "DOM.Iterable"] } } In summary, DOM is not implicitly "bundled" with ESNext in a way that persists if you choose to override the lib setting [1][5]. If lib is left undefined, TypeScript manages the defaults based on the target [3][1], but once you provide a custom lib list, you are responsible for including DOM if it is needed [5].
Citations:
- 1: https://microsoft.github.io/TypeScript-New-Handbook/reference/compiler-options/
- 2: https://www.typescriptlang.org/docs/handbook/2/type-declarations
- 3: https://stackoverflow.com/questions/63943629/what-is-the-typescript-compilers-default-lib-value
- 4:
--target ESNextdoes not include lib.dom.d.ts by default microsoft/TypeScript#14401 - 5: Intersection of features in modular lib files microsoft/TypeScript#11093
🌐 Web query:
Angular TSConfig ESNext includes DOM DOM.Iterable
💡 Result:
In TypeScript 6.0 and later, the contents of the DOM.Iterable and DOM.AsyncIterable libraries are fully included in the DOM library by default [1]. Consequently, if you are using TypeScript 6.0 or newer, explicitly including DOM.Iterable in your tsconfig.json lib array is no longer required and is considered redundant [1]. Prior to TypeScript 6.0, DOM.Iterable was a separate library that needed to be explicitly added to the lib configuration in tsconfig.json to enable iteration methods (such as for...of loops) on DOM collections like NodeList or HTMLCollection [1][2]. If you are working on an Angular project, your tsconfig.json configuration should align with the version of TypeScript being used: 1. For TypeScript 6.0+: You only need to include "DOM" in your lib array to support standard DOM APIs and their iterable counterparts [1]. 2. For versions prior to TypeScript 6.0: You must include both "DOM" and "DOM.Iterable" if your code performs iteration over DOM elements (e.g., document.querySelectorAll(...)) to avoid type errors [2][3]. The ESNext library option is often included alongside DOM to provide access to the latest ECMAScript features supported by the TypeScript compiler [4][5]. While ESNext includes modern JS APIs, it does not automatically include DOM-specific definitions, which is why DOM must be specified separately [4]. Summary of Key Points: - The separation between DOM and DOM.Iterable was a historical convenience for environments that did not support newer JavaScript iteration features [6]. - TypeScript 6.0 eliminated this separation to simplify configuration for modern development environments [1]. - You can safely remove "DOM.Iterable" from your tsconfig.json if your project is upgraded to TypeScript 6.0 or higher [1].
Citations:
- 1: https://devblogs.microsoft.com/typescript/announcing-typescript-6-0/
- 2: https://stackoverflow.com/questions/69600679/in-typescript-config-file-tsconfig-json-is-it-redundant-to-set-lib-compiler
- 3: https://www.totaltypescript.com/workshops/typescript-pro-essentials/types-you-don't-control/update-dom-typing-to-include-iterable/solution
- 4: https://www.typescriptlang.org/tsconfig/lib.html
- 5: https://www.typescriptlang.org/tsconfig/
- 6: https://stackoverflow.com/questions/71510368/why-are-dom-and-dom-iterable-separate
🌐 Web query:
TypeScript 6.0 source lib list ESNext includes DOM DOM.Iterable
💡 Result:
In TypeScript 6.0, released on March 23, 2026, the contents of the dom.iterable and dom.asynciterable library files have been fully integrated into the standard dom library [1][2]. As a result, if you specify "lib": ["dom"] in your tsconfig.json, you no longer need to manually include dom.iterable or dom.asynciterable to use iteration methods on DOM collections like NodeList or HTMLCollection [2]. While you can still explicitly list them in your configuration file, doing so is now redundant as they effectively function as empty files [2]. Regarding ESNext and its relationship with DOM libraries: The ESNext library option is distinct and provides type definitions for the latest ECMAScript features (as the specification evolves) [3][4]. The integration mentioned above specifically affects the dom library, ensuring that modern web standards are consistently available by default without requiring additional manual library imports [2]. Top Results: [5][1][2]
Citations:
- 1: https://visualstudiomagazine.com/articles/2026/03/23/typescript-6-0-ships-as-final-javascript-based-release-clears-path-for-go-native-7-0.aspx
- 2: https://devblogs.microsoft.com/typescript/announcing-typescript-6-0-rc/
- 3: https://www.typescriptlang.org/tsconfig/lib.html
- 4: https://microsoft.github.io/TypeScript-New-Handbook/reference/compiler-options/
- 5: https://devblogs.microsoft.com/typescript/announcing-typescript-6-0/
🌐 Web query:
TypeScript 6.0 compilerOptions lib ESNext automatically includes DOM
💡 Result:
The TypeScript compiler does not automatically include DOM type definitions when "target" is set to "ESNext" [1]. TypeScript infers a default set of library files ("lib") based on the "target" compiler option [2][3]. While older "target" values (like "ES5" or "ES6") may have historically included "DOM" by default in some configurations, setting "target" to "ESNext" does not guarantee the inclusion of the "DOM" library [2][1][4]. When you explicitly define the "lib" option in your tsconfig.json, you completely override the default library inclusion behavior [5][4]. Therefore, if you require DOM support (e.g., access to window, document) while using "ESNext", you must explicitly add "DOM" to your "lib" array [5][3][6]: { "compilerOptions": { "target": "ESNext", "lib": ["ESNext", "DOM"] } } In modern TypeScript development, it is considered best practice to explicitly specify your "lib" configuration rather than relying on the implicit defaults determined by the "target" version, as this makes your environment requirements predictable and avoids unintended inclusions [2][3][6].
Citations:
- 1:
--target ESNextdoes not include lib.dom.d.ts by default microsoft/TypeScript#14401 - 2: https://microsoft.github.io/TypeScript-New-Handbook/reference/compiler-options/
- 3: https://runebook.dev/en/docs/typescript/tsconfig/lib-config
- 4: Intersection of features in modular lib files microsoft/TypeScript#11093
- 5: https://learntypescript.dev/11/l5-libs/
- 6: https://www.totaltypescript.com/books/total-typescript-essentials/configuring-typescript
🌐 Web query:
angular-devkit/build-angular TSConfig base json esnext lib includes DOM
💡 Result:
In TypeScript and Angular projects, the lib compiler option defines the set of built-in type definitions that are included in the compilation [1]. Including DOM in the lib array is essential for Angular applications because it provides type definitions for browser-based APIs such as window, document, and localStorage [1][2]. Regarding the use of ESNext and DOM in your tsconfig.json: 1. Role of DOM: The DOM library is required for any application that interacts with browser interfaces [1]. Without it, TypeScript will fail to recognize standard browser global objects, leading to errors like "Cannot find name 'document'" [1][2]. 2. Configuration and Defaults: Historically, Angular CLI projects explicitly specified lib values (e.g., ["ES2022", "DOM"]) in the generated tsconfig.json [3][4]. However, recent versions of the Angular CLI have refactored this to remove explicit lib declarations in new projects [5]. When lib is not explicitly defined, TypeScript defaults to including DOM and the type definitions matching your target option [1][5]. 3. ESNext vs. Specific Versions: While you may see ESNext used in some configurations, standard Angular projects typically use specific ECMAScript versions (such as ES2022) that correspond to the browser compatibility requirements managed by the browserslist configuration [3][4]. Using ESNext provides the latest, most cutting-edge JavaScript features, but it can potentially introduce type definitions for APIs that are not yet stable or widely supported in your target runtime environment [1]. In summary, including DOM is standard practice for frontend applications, while the choice between ESNext and a versioned standard (like ES2022) depends on your specific requirement for bleeding-edge versus stable, supported API definitions [1][4]. Most Angular developers should rely on the default behavior or explicitly specify a stable version that aligns with their target [3][5].
Citations:
- 1: https://www.typescriptlang.org/tsconfig/lib.html
- 2: https://stackoverflow.com/questions/51883220/what-is-dom-for-in-the-lib-array-in-the-tsconfig-json-file-in-angular
- 3: https://v17.angular.io/guide/typescript-configuration
- 4: angular/angular-cli@1e5d4a7
- 5: angular/angular-cli@85f75aa
🌐 Web query:
TypeScript lib option default target ESNext includes DOM Angular TSConfig lib ESNext DOM
💡 Result:
In TypeScript, the lib compiler option determines which built-in JavaScript API type definitions are available for your project [1]. By default, this set is automatically derived based on your target option; if lib is not explicitly set, TypeScript injects a default list corresponding to your target [2][3]. The ESNext value for lib includes all ECMAScript APIs available in the latest version of the JavaScript specification supported by TypeScript [1][4]. It does not, however, include browser or environment-specific APIs (like the DOM) by default [1]. Key points regarding your query: 1. Relationship between Target and Lib: If you set your target to ESNext, TypeScript does not automatically include the DOM lib unless it is explicitly specified or your target defaults happen to include it [1][4]. Because ESNext represents the bleeding edge, developers typically must explicitly define both the ECMAScript version and the environment (e.g., "lib": ["ESNext", "DOM"]) in their tsconfig.json to ensure all necessary types are available [3]. 2. Angular Configuration: Angular projects typically manage these settings in the workspace-level tsconfig.json or project-specific files like tsconfig.app.json [5]. It is standard practice in Angular to explicitly list both the ECMAScript target and the DOM library in the lib array (e.g., "lib": ["ES2022", "DOM"]) to ensure consistent type checking across the browser-based environment [5]. 3. ESNext Caution: While ESNext provides access to the latest features, it is a moving target [4]. Because its contents change as the JavaScript specification evolves and as new TypeScript versions are released, using it can sometimes lead to less predictable upgrades compared to using a specific versioned lib like ES2022 or ES2023 [4]. In summary, if you intend to use ESNext features in a browser-based Angular application, you should explicitly include both ESNext and DOM in your tsconfig.json lib array to ensure full type support [3].
Citations:
- 1: https://www.typescriptlang.org/tsconfig/lib.html
- 2: https://stackoverflow.com/questions/63943629/what-is-the-typescript-compilers-default-lib-value
- 3: https://learntypescript.dev/11/l5-libs/
- 4: https://microsoft.github.io/TypeScript-New-Handbook/reference/compiler-options/
- 5: https://v17.angular.io/guide/typescript-configuration
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
pnpm ls --depth=0 | sed -n '1,120p'
python3 - <<'PY'
import json, pathlib, re, tarfile, tempfile, urllib.request, zipfile
p=json.loads(open('package.json', encoding='utf-8').read())
ver=p['devDependencies']['typescript']
print('typescript version from package.json:', ver)
PYRepository: VaibhavDaveDev/Meterly
Length of output: 1156
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
tmp="$(mktemp -d)"
cd "$tmp"
version=$(node - <<'JS'
import fs from 'node:fs';
const pkg=JSON.parse(fs.readFileSync('../package.json','utf8'));
console.log(pkg.devDependencies.typescript);
JS
rm -rf "../$tmp"
if [ -z "$version" ]; then echo "typescript version unknown"; exit 0; fi
echo "typescript version: $version"
npm pack typescript@$version --silent > /tmp/ts-version-pack-a
echo "downloaded package tar ball"
tar -tzf /tmp/ts-version-pack-a | rg 'tsc\.js|types/lib' | sed -n '1,80p'
python3 - <<'PY'
import json, tarfile, re, sys
name='typescript-'+sys.argv[1]+'.tgz'
print('searching', name)
with tarfile.open('/tmp/ts-version-pack-a','r:gz') as tar:
names=[n for n in tar.getnames() if n.endswith('lib.esnext.d.ts') or n.endswith('lib.dom.d.ts') or re.search(r'(^|/)lib\.esnext.*(\.d\.ts)$|(^|/)lib\.dom.*(\.d\.ts)$', n)]
print(names[:50])
PY
PY "$version"
tmp="$(mktemp -d)"
cd "$tmp"
version=$(node - <<'JS'
import fs from 'node:fs';
const pkg=JSON.parse(fs.readFileSync('package.json','utf8'));
console.log(pkg.devDependencies.typescript);
JS
)
echo "typescript version: $version"
npm pack typescript@$version --silent > /tmp/ts-version-pack-b
temp_pkg="/tmp/ts-version-pack-b"
python3 - <<'PY'
import json, tarfile, re, textwrap
with tarfile.open(sys.argv[1], 'r:gz') as tar:
f=tar.extractfile('package/lib/lib.esnext.d.ts')
if f:
print('--- package/lib/lib.esnext.d.ts ---')
print(f.read().decode('utf8', errors='replace')[:1400])
PY "$temp_pkg"Repository: VaibhavDaveDev/Meterly
Length of output: 309
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
version=$(node -e 'import fs from "node:fs"; const pkg=JSON.parse(fs.readFileSync("package.json","utf8")); console.log(pkg.devDependencies.typescript)')
echo "typescript version: $version"
npm pack typescript@$version --silent > /tmp/ts-version-pack.tgz
echo "package name:"
tar -tzf /tmp/ts-version-pack.tgz | sed -n '1,3p'
echo
echo "--- package/lib/lib.esnext.d.ts (first 90 lines) ---"
python3 - <<'PY'
import tarfile, sys
with tarfile.open("/tmp/ts-version-pack.tgz", "r:gz") as tar:
f = tar.extractfile("package/lib/lib.esnext.d.ts")
if f:
print(f.read().decode().split("\n", 89)[0] + "\n".join(f.read().decode().split("\n", 89)[1:89]))
PY
echo
echo "--- package/lib/lib.dom.d.ts first include/import lines ---"
python3 - <<'PY'
import tarfile, re
with tarfile.open("/tmp/ts-version-pack.tgz", "r:gz") as tar:
f = tar.extractfile("package/lib/lib.dom.d.ts")
if f:
text = f.read().decode()
for i, line in enumerate(text.splitlines(), 1):
if i > 40 or re.search(r"^\s*///\s*<reference\s+lib=(?:string|none)|^\s*import", line):
print(f"{i}: {line}")
PYRepository: VaibhavDaveDev/Meterly
Length of output: 469
🌐 Web query:
github microsoft TypeScript lib.esnext.d.ts "lib.dom" "ESNext"
💡 Result:
In TypeScript, lib.esnext.d.ts and lib.dom are standard library declaration files that provide type definitions for JavaScript built-in APIs and web platform APIs [1]. These files allow the TypeScript compiler to type-check your code against available global environments [2][1]. lib.esnext.d.ts This file is a "rolling" library definition that contains type declarations for the latest, upcoming ECMAScript features [2]. Because the ECMAScript specification is constantly evolving, the contents of esnext change as new features are added to the language specification [2]. In modern TypeScript (e.g., 6.0+), it acts as an aggregate that references various granular esnext.* library files, such as: - esnext.collection - esnext.decorators - esnext.temporal - esnext.array - esnext.promise These granular files allow developers to opt into specific future features without including the entire, potentially unstable set of esnext definitions [3][4]. lib.dom This library contains type definitions for browser-based APIs (e.g., window, document, fetch) [2][1]. As of TypeScript 6.0, the lib.dom library has been simplified: it now fully includes the definitions that were previously found in the separate lib.dom.iterable.d.ts and lib.dom.asynciterable.d.ts files [5][6]. You no longer need to manually specify those extra libraries in your tsconfig.json; including "dom" is sufficient [5]. Usage You configure these libraries in your tsconfig.json using the lib compiler option: { "compilerOptions": { "lib": ["esnext", "dom"] } } Note that if you specify the lib option, it replaces the default set of libraries that would otherwise be selected based on your target setting [2][1]. If you want to use the default libraries plus specific additions, you must manually list all required libraries (e.g., ["es2022", "dom", "esnext.temporal"]) [1]. Additionally, you can use triple-slash directives in individual files to reference these libraries, which is useful for declaration file authors [7][8]: /// ///
Citations:
- 1: https://deepwiki.com/microsoft/TypeScript/6-standard-library-type-definitions
- 2: https://www.typescriptlang.org/tsconfig/lib.html
- 3: https://github.com/microsoft/TypeScript/blob/master/src/lib/esnext.d.ts
- 4: https://github.com/microsoft/TypeScript/blob/main/src/lib/libs.json
- 5: https://devblogs.microsoft.com/typescript/announcing-typescript-6-0/
- 6: https://lilting.ch/en/articles/typescript-6-rc
- 7: https://www.typescriptlang.org/docs/handbook/release-notes/typescript-4-2.html
- 8: Using /// <reference lib="dom" /> in a .d.ts typescript declaration file has global effects microsoft/TypeScript#33901
Restore DOM declarations for the shared TypeScript configuration.
"lib": ["ESNext"] does not include browser declarations, but many shared files use DOM globals such as window, document, HTMLDivElement, and HTMLInputElement. Add DOM to the library list, or split browser-specific code into a config that loads DOM declarations.
Proposed fix
- "lib": ["ESNext"],
+ "lib": ["ESNext", "DOM"],📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "lib": ["ESNext"], | |
| "lib": ["ESNext", "DOM"], |
Source: MCP tools
1 participant
Security hardening, upload validation, and polyglot payload protection
Summary by CodeRabbit
Security
Bug Fixes
Documentation
Tests