Skip to content

fix(admin): fall back to Bearer JWT when CF Access is disabled but env vars remain#2342

Closed
andrew-bierman wants to merge 55 commits into
mainfrom
fix/admin-basic-auth-fallback
Closed

fix(admin): fall back to Bearer JWT when CF Access is disabled but env vars remain#2342
andrew-bierman wants to merge 55 commits into
mainfrom
fix/admin-basic-auth-fallback

Conversation

@andrew-bierman
Copy link
Copy Markdown
Collaborator

@andrew-bierman andrew-bierman commented Apr 27, 2026

Summary

  • adminAuthGuard now falls back to Bearer JWT check when CF_ACCESS_TEAM_DOMAIN/CF_ACCESS_AUD are set but the request has no CF Access JWT
  • Previously, any request without a valid CF Access JWT was immediately rejected with 401 when CF env vars were configured — breaking Basic-auth login after disabling CF Access on the dashboard

Root Cause

When a user disables CF Access from the Cloudflare dashboard (but hasn't yet cleared the API env vars), requests arrive without a CF JWT. The guard treated the presence of env vars as a hard requirement for CF auth, ignoring the admin Bearer JWT issued by /token.

Behaviour After Fix

Scenario Before After
CF Access active, CF JWT present ✅ allowed ✅ allowed
CF Access disabled, env vars still set, Bearer JWT ❌ 401 ✅ allowed
No CF vars, Bearer JWT ✅ allowed ✅ allowed
No CF vars, Basic auth ✅ allowed ✅ allowed

Test plan

  • Log in via Basic auth while CF_ACCESS_TEAM_DOMAIN/CF_ACCESS_AUD are set but no CF Access cookie is present — should succeed
  • Log in via Basic auth with no CF env vars — still works
  • With CF Access active, CF JWT still accepted

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added trail search and discovery functionality with spatial filtering and sport-based categorization
  • Improvements

    • Enhanced environment variable validation with centralized schemas
    • Strengthened authentication and access control logic
    • Improved type safety and error handling throughout the application
  • Chores

    • Extended CI/CD pipeline with additional custom linting checks and type-cast validation

andrew-bierman and others added 30 commits April 26, 2026 13:34
…ery builder

- New packages/overpass package: typed Overpass QL builder, HTTP client,
  GeoJSON transform (TrailSummary / TrailDetail), full re-export index
- packages/api: GET /api/trails/search, /api/trails/:osmId, /api/trails/:osmId/geometry
- packages/mcp: search_trails, get_trail, get_trail_geometry tools
…nctions

33 tests covering all sport mappings, spatial filters, OQL output structure,
geometry assembly (LineString/MultiLineString), and tag field fallbacks.
…e-guards

Pre-push was a dead zone — lefthook only ran Biome on pre-commit, so
all 9 custom scripts were invisible to developers. This PR closes that
gap and adds a missing check.

- lefthook: add pre-push hook running `bun check:all` (full custom suite)
- checks.yml: add lint:custom and check:casts:strict steps (continue-on-error
  while existing backlog is cleared; pre-push already blocks new violations)
- biome.json: escalate useTopLevelRegex warn → error
- package.json: expose `check:all` as a root script
- scripts/check-all.ts: add no-raw-process-env, no-duplicate-guards,
  check-type-casts --strict to the orchestrated suite
- scripts/lint/no-duplicate-guards.ts (new): flags re-implementations of
  guards already exported from @packrat/guards; found 4 assertDefined
  duplicates on first run
- Fix EXCLUDED_ROOTS trailing-slash mismatch (dirs were not excluded early)
- Add no-duplicate-guards to lint:custom so CI covers it
- Fix check-all.ts comment: correct path for no-raw-process-env
- Fix header comment: wired into lint:custom, not checks.yml directly
…, fix bbox bounds, add auth

- builder.ts: escape backslashes before double-quotes in name() to prevent Overpass QL injection
- transform.ts: normalize OSM route tag (bicycle, ski) to OsmSport values (cycling, skiing)
- transform.test.ts: fix swapped minlon/maxlon in PCT bounds fixture and bbox assertions
- trails/index.ts: add authPlugin + isAuthenticated to all trail endpoints
…eader

Replace default redirect-following fetch with manual redirect handling.
Validates the Location header hostname against the alltrails.com allowlist
before issuing a second fetch, preventing server-side request forgery via
malicious redirect chains.
feat(api): AllTrails OG preview endpoint
chore(checks): wire custom checks into pre-push + CI, add no-duplicate-guards
window, document, globalThis, Bun, navigator, process are
environment/runtime availability checks — accessing them as
undeclared globals would ReferenceError, so they cannot be
replaced with isDefined() guards.
…ards

Four packages had hand-rolled assertDefined that could diverge silently.
Converted each to a barrel re-export so existing import paths are
preserved but the implementation lives in one place.
analytics, api-client, cli, and config were importing guards
for the first time; workspace:* pins them to the monorepo copy.
73 violations cleared across apps/expo, apps/landing, apps/guides:
- isString / isNumber / isFunction / isObject from @packrat/guards
  replace typeof === 'string' / 'number' / 'function' / 'object'
- typeof ref !== 'function' (callback-ref pattern) → !isFunction(ref)
- typeof item.id === 'string' multi-field guards → isString(item.id) etc.
Covers api, api-client, analytics, cli, config, web-ui:
- isString / isNumber / isFunction / isObject replace typeof checks
- r2-bucket.ts guard function, CatalogItemValidator, weather routes,
  chart/sidebar components, env-validation, csv-utils all updated
- typeof globalThis (env-validation:127) and typeof Bun (smoke-test)
  left as-is — covered by the global-identifier exemption in the check
check-all runs strict on everything, which blocks mid-backlog pushes.
Gate pre-push only on currently-passing checks; CI still reports the
remaining backlog (no-raw-regex, no-raw-process-env, check-type-casts)
via continue-on-error. Re-add each here as its backlog clears.
Both apps re-export assertDefined from @packrat/guards but didn't declare
the workspace dep — builds relied on transitive hoisting.
Adds 5 entries to the allowlist (the check script itself, api startup
validator, analytics env module, guides sync script, api test directory)
and introduces apps/admin/lib/env.ts so NEXT_PUBLIC_API_URL is typed
via Zod rather than read raw.
format-ai-response (expo + api), csv-utils, enhance-content,
chart.tsx, mcp tools, api test setup — 40 patterns extracted.
Regex now compiles once and is self-documenting at the call site.
reader.ts (29 HTML→Markdown patterns), trailConditions, wildlife,
packTemplates, r2-bucket, embeddingService, executeSqlAiTool,
entity-resolver, data-export — 44 patterns extracted.
no-raw-regex now passes clean (0 violations).
Both checks now pass clean — move them from continue-on-error CI
and add them to the pre-push gate. Only check-type-casts remains
in the backlog (130 casts, next PR).
- Rename TIMESTAMP_SAFE_CHARS → TIMESTAMP_UNSAFE_CHARS (matches its purpose)
- Fix CURLY_SINGLE_QUOTES: add U+2018/U+2019 which were missing
- Fix CURLY_DOUBLE_QUOTES: add U+201C/U+201D which were missing
- Fix normalizeJsonString: replace with straight quotes (was replacing with
  smart quotes U+2019/U+201D, which would still break JSON.parse)
…e-positive fixes

- Exclude test files (/__tests__/, /test/) from cast scanning
- Extend safe-cast annotation lookback through consecutive comment lines
  (was 1 line; now scans the entire comment block preceding the cast)
- Add NATURAL_LANGUAGE_CAST pattern to skip content-string false positives
- Add ArrayBuffer, ReadableStream, SQL to SAFE_CAST_PATTERNS
…ditions

- computePackWeights: remove redundant WeightUnit cast
- PackDetailScreen/PackCard: annotation for Pack union narrowing
- usePackTemplateSummary: remove redundant casts
- TrailConditionReportCard: remove redundant casts
- treaty hooks: annotate with safe-cast where response shape matches schema
…ps, components)

- treaty response hooks: safe-cast annotations for shape-match casts
- ToolInvocationRenderer: inline safe-cast on each discriminated case branch
- Icon components: safe-cast for ComponentProps/SfSymbolName icon type coercions
- store.ts obs(): safe-cast for Legend-State Proxy access pattern
- guides generate page: type ContentCategory at Object.entries; DifficultyLevel uses assertEnum
- guides sync-to-r2: inline safe-cast for SyncEnv → ValidatedEnv subset
- landing icons: safe-cast for Lucide icon type narrowing
…, config, web-ui)

- env-validation: safe-cast annotations for Cloudflare Worker runtime bindings
- api middleware/services: safe-cast for treaty/generic boundary casts
- analytics: safe-cast for DuckDB row result types
- guards narrow.ts: assertEnum helper for enum type narrowing
- cli, config, web-ui: safe-cast for legitimate type boundary casts
andrew-bierman and others added 14 commits April 26, 2026 16:03
Treaty response type has createdAt as string | undefined but the
PackItem schema type requires string; the cast bridges this gap.
The /alltrails/preview endpoint is a link-preview proxy that fetches
OG metadata from AllTrails. No user data is accessed or modified,
so it intentionally omits auth macros.
feat: cross-file magic string detection
feat(overpass): Typed Overpass API query builder and trail routes
chore: clear type-cast backlog and promote to hard gate
Remove the public-route annotation and add isAuthenticated: true.
The link-preview proxy should only be callable by authenticated users
to prevent abuse as an open AllTrails scraping proxy.
Backslash and double-quote escape patterns moved to module scope.
client.ts: casts validated by isObject + Array.isArray guard
transform.ts: way geometry cast per Overpass API contract
…pe predicates

Add OverpassResponseSchema for runtime validation in queryOverpass, eliminating
the need for any type casts. Use a type predicate filter in toTrailGeometry so
TypeScript can narrow the geometry array without casting.
…s route

Destructure array access results so TypeScript doesn't widen the type to
T | undefined under noUncheckedIndexedAccess (root tsconfig enables this).
fix: require auth on alltrails preview route
Copilot AI review requested due to automatic review settings April 27, 2026 00:18
@github-actions github-actions Bot added dependencies Pull requests that update a dependency file api ci/cd mobile web labels Apr 27, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 27, 2026

Warning

Rate limit exceeded

@andrew-bierman has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 48 minutes and 32 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 48 minutes and 32 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6cc5c583-0ff7-4597-aaf7-0ed75b5c51dd

📥 Commits

Reviewing files that changed from the base of the PR and between 43e613c and 646e485.

📒 Files selected for processing (1)
  • packages/api/src/routes/admin/index.ts

Warning

.coderabbit.yaml has a parsing error

The CodeRabbit configuration file in this repository has a parsing error and default settings were used instead. Please fix the error(s) in the configuration file. You can initialize chat with CodeRabbit to get help with the configuration file.

💥 Parsing errors (1)
Validation error: String must contain at most 250 character(s) at "tone_instructions"
⚙️ Configuration instructions
  • Please see the configuration documentation for more information.
  • You can also validate your configuration using the online YAML validator.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json
📝 Walkthrough

Walkthrough

This PR establishes centralized type-guard utilities and migrates the codebase to use them, introduces the @packrat/overpass package for trail data integration, adds trail search/detail API routes backed by Overpass, hardens AllTrails redirect validation with authentication enforcement, creates a centralized admin environment validation layer, extracts regex patterns into constants, and expands linting checks for guard duplicates and unauthenticated routes.

Changes

Cohort / File(s) Summary
Guard Migration & Type Safety
apps/expo/**, apps/guides/**, apps/landing/**, packages/api/src/**, packages/analytics/**, packages/api-client/**, packages/cli/**, packages/config/**, packages/web-ui/**
Widespread replacement of inline typeof checks with @packrat/guards predicates (isString, isFunction, isObject, isNumber); inline comments added documenting safe-cast assumptions at API boundaries and type narrowing locations.
New Overpass Package
packages/overpass/src/builder.ts, packages/overpass/src/client.ts, packages/overpass/src/schemas.ts, packages/overpass/src/transform.ts, packages/overpass/src/types.ts, packages/overpass/src/*.test.ts, packages/overpass/package.json, packages/overpass/tsconfig.json, packages/overpass/vitest.config.ts
Complete new @packrat/overpass package providing Overpass QL query builder, HTTP client with response validation, Zod schemas, and transformations to convert Overpass elements into TrailSummary/TrailDetail/geometry shapes; includes comprehensive test coverage.
Trail API Routes & MCP Tools
packages/api/src/routes/trails/index.ts, packages/mcp/src/tools/trails.ts
New authenticated /trails route group with /search (name/spatial/sport filtering with pagination), /:osmId (detail), and /:osmId/geometry endpoints; new MCP tools for trail search and detail fetching via the API.
AllTrails Route Security Hardening
packages/api/src/routes/alltrails.ts
Preview route now enforces isAuthenticated: true, disables automatic redirect following, validates Location header presence, enforces https: and alltrails.com hostname via regex, handles Overpass/fetch timeouts as 504, and resolves absolute URLs before second fetch.
Admin App Centralized Env
apps/admin/lib/env.ts, apps/admin/app/login/page.tsx, apps/admin/lib/api.ts
New env-validation module using Zod to validate NEXT_PUBLIC_API_URL at module load; login page and API client now source URL from adminEnv instead of direct process.env.
Regex & Pattern Extraction
packages/api/src/routes/**, packages/analytics/**, packages/api/src/services/**, packages/cli/**, packages/mcp/**, packages/web-ui/**, scripts/**
Module-level regex constants replace repeated inline patterns for newline normalization, hyphen stripping, whitespace collapsing, SQL join matching, quote escaping, and slug sanitization.
Linting & Type Checking
.github/workflows/checks.yml, scripts/lint/no-raw-typeof.ts, scripts/lint/no-raw-regex.ts, scripts/lint/no-duplicate-guards.ts, scripts/lint/no-unauth-routes.ts, packages/checks/src/check-type-casts.ts, scripts/check-all.ts, lefthook.yml, biome.json
CI workflow now runs bun lint:custom with expanded checks; new linting scripts for duplicate guards and unauthenticated routes; check-type-casts now supports // safe-cast: inline annotations and broader safe-cast whitelist; biome severity for useTopLevelRegex changed to error; pre-push hook added.
Dependency Updates
apps/admin/package.json, apps/expo/package.json, apps/guides/package.json, apps/landing/package.json, packages/analytics/package.json, packages/api-client/package.json, packages/api/package.json, packages/cli/package.json, packages/config/package.json, packages/web-ui/package.json
Addition of @packrat/guards (and @packrat/overpass to API package) as workspace dependencies across multiple apps and packages.
Local Type Assertions → Centralized Re-exports
apps/guides/lib/assertDefined.ts, apps/landing/lib/typeAssertions.ts, packages/analytics/src/core/type-assertions.ts, packages/web-ui/src/lib/utils.ts
Locally defined assertDefined helpers replaced with re-exports from @packrat/guards, consolidating type-assertion implementation.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant API
    participant Overpass
    participant TrailDB
    
    Client->>API: GET /trails/search?name=...&lat=...&lon=...&sport=...
    activate API
    API->>API: Validate query params (Zod)
    API->>API: TrailQueryBuilder.build() QL
    API->>Overpass: POST query (route relation with filters)
    activate Overpass
    Overpass->>Overpass: Parse filters, execute spatial query
    Overpass-->>API: JSON elements array
    deactivate Overpass
    API->>API: OverpassResponseSchema.parse() validation
    API->>API: Map elements → TrailSummary[]
    API-->>Client: 200 + Trail[] (name, sport, distance, difficulty, bbox)
    deactivate API
    
    alt Detail Request
        Client->>API: GET /trails/{osmId}
        activate API
        API->>API: Validate osmId
        API->>Overpass: Query single relation by ID
        Overpass-->>API: Element with geometry
        API->>API: toTrailDetail(element) → summary + geometry
        API-->>Client: 200 + TrailDetail (LineString/MultiLineString GeoJSON)
        deactivate API
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related PRs

  • feat(api): AllTrails OG preview endpoint and MCP tool #2310: Both PRs modify packages/api/src/routes/alltrails.ts; this PR hardens redirect validation and enforces authentication on the preview route.
  • Development -> main #2299: Both PRs update the admin app's environment and API client setup (apps/admin/lib/env.ts, apps/admin/lib/api.ts), migrating to centralized env validation.
  • Release v2.0.15 #1979: Both PRs introduce/enhance the @packrat/overpass package with builder, client, schemas, and transform modules for trail data querying.

Suggested labels

api, refactoring, type-safety, security

Suggested reviewers

  • Isthisanmol
  • mikib0

Poem

🐰 Type guards hop through the code so bright,
Overpass trails lead to the height,
Safe-casts approved, regexes named,
Admin envs tamed, AllTrails reclaimed!
One guard to rule them all,

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 17.65% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: updating the admin auth guard to fall back to Bearer JWT when CF Access is disabled. The change is directly reflected in the PR objectives and commit message.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/admin-basic-auth-fallback

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 27, 2026

Coverage Report for Expo Unit Tests Coverage (./apps/expo)

Status Category Percentage Covered / Total
🔵 Lines 81.4% 521 / 640
🔵 Statements 81.4% (🎯 75%) 521 / 640
🔵 Functions 92.85% 52 / 56
🔵 Branches 92.55% 199 / 215
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
apps/expo/features/packs/utils/computePackWeights.ts 100% 100% 100% 100%
apps/expo/lib/utils/dateUtils.ts 0% 0% 0% 0% 1-34
apps/expo/lib/utils/domain-specific-extensions.ts 100% 87.5% 100% 100%
apps/expo/utils/format-ai-response.ts 100% 100% 100% 100%
Generated in workflow #860 for commit 646e485 by the Vitest Coverage Report Action

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 27, 2026

Coverage Report for API Unit Tests Coverage (./packages/api)

Status Category Percentage Covered / Total
🔵 Lines 75.74% 609 / 804
🔵 Statements 75.74% (🎯 65%) 609 / 804
🔵 Functions 95.91% 47 / 49
🔵 Branches 88.23% 270 / 306
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
packages/api/src/services/embeddingService.ts 100% 100% 100% 100%
packages/api/src/services/refreshTokenService.ts 0% 100% 100% 0% 2-176
packages/api/src/utils/auth.ts 98.93% 90.9% 100% 98.93% 52
packages/api/src/utils/compute-pack.ts 91.37% 76.19% 100% 91.37% 10, 14, 16, 30, 32
packages/api/src/utils/csv-utils.ts 95.43% 87.5% 100% 95.43% 66-67, 69-70, 88-89, 132-133, 149-150, 171-172
packages/api/src/utils/format-ai-response.ts 100% 100% 100% 100%
Generated in workflow #860 for commit 646e485 by the Vitest Coverage Report Action

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates admin authentication to better handle Cloudflare Access being disabled while env vars remain set, and adds a broad set of repo-wide linting/guard consistency changes plus new trail-search functionality.

Changes:

  • Adjust adminAuthGuard to allow admin Bearer JWT auth when CF Access env vars are set but no CF Access JWT is present.
  • Add a new @packrat/overpass package + authenticated /trails API routes and corresponding MCP tools.
  • Introduce/extend custom lint/check scripts (unauthenticated routes, duplicate guards, stricter top-level regex) and refactor many occurrences to use @packrat/guards and hoisted regex constants.

Reviewed changes

Copilot reviewed 145 out of 146 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
scripts/lint/no-unauth-routes.ts New custom lint to detect potentially public Elysia routes missing auth macros/annotations.
scripts/lint/no-raw-typeof.ts Enhances typeof lint rule with exemptions for safe SSR/global checks.
scripts/lint/no-raw-regex.ts Expands allowlist for files permitted to use raw regex.
scripts/lint/no-duplicate-guards.ts New lint to prevent re-implementing guard helpers outside @packrat/guards.
scripts/check-all.ts Adds additional checks (process.env, duplicate guards, unauth routes, strict cast checks).
packages/web-ui/src/lib/utils.ts Re-exports assertDefined from @packrat/guards.
packages/web-ui/src/components/sidebar.tsx Replaces raw typeof checks with @packrat/guards helpers.
packages/web-ui/src/components/form.tsx Converts sentinel context casts to documented “safe-cast” style.
packages/web-ui/src/components/chart.tsx Hoists regex constants and replaces raw typeof/object checks with guards.
packages/overpass/vitest.config.ts Adds Vitest config for new Overpass package tests.
packages/overpass/tsconfig.json Adds TS config for new Overpass package.
packages/overpass/src/types.ts Defines Overpass API response types and OSM sport union.
packages/overpass/src/transform.ts Adds transformers from Overpass elements → trail summaries/details/geometry.
packages/overpass/src/transform.test.ts Unit tests for Overpass transformers.
packages/overpass/src/schemas.ts Adds Zod schema for validating Overpass responses.
packages/overpass/src/index.ts Public exports for the Overpass package.
packages/overpass/src/client.ts Fetch + validate Overpass queries with Zod.
packages/overpass/src/builder.ts Fluent builder for Overpass QL trail queries.
packages/overpass/src/builder.test.ts Unit tests for the Overpass query builder.
packages/overpass/package.json Registers @packrat/overpass workspace package and scripts.
packages/mcp/src/tools/trips.ts Hoists regex constant used in ID generation.
packages/mcp/src/tools/trails.ts Adds MCP tools for searching/fetching trails and geometry via API.
packages/mcp/src/tools/trail-conditions.ts Hoists regex constant used in ID generation.
packages/mcp/src/tools/packs.ts Hoists regex constant used in ID generation.
packages/guards/src/narrow.ts Updates cast comment to “safe-cast” convention.
packages/env/scripts/no-raw-process-env.ts Expands allowlist for process.env usage in specific scripts/tests.
packages/config/src/config.ts Uses isObject guard and documents required cast in deepFreeze.
packages/config/package.json Adds @packrat/guards dependency.
packages/cli/src/shared.ts Uses isNumber guard and documents safe casts used for table printing.
packages/cli/src/args.ts Uses isString guard in Zod preprocessors.
packages/cli/package.json Adds @packrat/guards dependency.
packages/checks/src/check-type-casts.ts Extends exclusions and adds // safe-cast: annotation support.
packages/checks/src/check-magic-strings.ts Reworks detection to be cross-file and hoists ignore patterns.
packages/api/test/setup.ts Hoists markdown-extension regex constant.
packages/api/src/utils/format-ai-response.ts Hoists formatting regex patterns to comply with Biome rule.
packages/api/src/utils/env-validation.ts Uses isObject guard; adds/standardizes safe-cast annotations.
packages/api/src/utils/csv-utils.ts Hoists CSV sanitization regex and replaces raw typeof checks with guards.
packages/api/src/utils/compute-pack.ts Removes unnecessary WeightUnit cast.
packages/api/src/utils/auth.ts Uses isNumber guard for expiration; documents JWT payload cast.
packages/api/src/services/refreshTokenService.ts Documents Drizzle SQL casts as safe-cast.
packages/api/src/services/r2-bucket.ts Uses guards (isObject, isString, isFunction) and hoists ETag regex.
packages/api/src/services/executeSqlAiTool.ts Hoists JOIN-count regex used for query complexity validation.
packages/api/src/services/etl/processValidItemsBatch.ts Converts explanatory block to inline safe-cast comment.
packages/api/src/services/etl/CatalogItemValidator.ts Replaces raw typeof checks with isString/isNumber guards.
packages/api/src/services/embeddingService.ts Hoists newline regex for embedding normalization.
packages/api/src/services/aiService.ts Uses isFunction guard for runtime binding checks.
packages/api/src/routes/wildlife/index.ts Hoists slug normalization regex patterns.
packages/api/src/routes/weather.ts Uses isString guard and documents API response casts.
packages/api/src/routes/trails/index.ts New authenticated trails endpoints backed by Overpass client.
packages/api/src/routes/trailConditions/reports.ts Hoists LIKE-escape regex patterns.
packages/api/src/routes/packs/index.ts Documents NewPackItem cast as safe-cast.
packages/api/src/routes/packTemplates/index.ts Hoists hyphen-stripping regex used in ID generation.
packages/api/src/routes/knowledgeBase/reader.ts Hoists HTML→Markdown conversion regex patterns.
packages/api/src/routes/index.ts Registers new trailsRoutes.
packages/api/src/routes/catalog/index.ts Uses isString guard for query param handling.
packages/api/src/routes/auth/index.ts Adds // public-route: annotations for unauth endpoints.
packages/api/src/routes/alltrails.ts Adds redirect validation (SSRF guard) and requires authentication.
packages/api/src/routes/ai/index.ts Uses isString guard on schema-info result.
packages/api/src/routes/admin/index.ts Admin auth guard now falls back to Bearer/Basic when CF JWT missing.
packages/api/src/middleware/auth.ts Documents casts to AuthUser as safe-cast.
packages/api/src/index.ts Documents Cloudflare worker entrypoint casts as safe-cast.
packages/api/src/containers/AppContainer.ts Documents runtime env cast as safe-cast.
packages/api/package.json Adds dependency on @packrat/overpass.
packages/api-client/src/index.ts Uses isString/isObject guards and documents fetch-boundary cast.
packages/api-client/package.json Adds @packrat/guards dependency.
packages/analytics/src/core/type-assertions.ts Re-exports assertDefined from @packrat/guards.
packages/analytics/src/core/spec-parser.ts Uses isString guard; documents safe casts from DuckDB results.
packages/analytics/src/core/local-cache.ts Documents generic cast boundary as safe-cast.
packages/analytics/src/core/entity-resolver.ts Hoists normalization regex patterns and uses isString guard.
packages/analytics/src/core/enrichment.ts Documents DuckDB result casts as safe-cast.
packages/analytics/src/core/data-export.ts Hoists timestamp sanitization regex.
packages/analytics/package.json Adds @packrat/guards dependency.
package.json Adds check:all and expands lint:custom to include new scripts.
lefthook.yml Adds pre-push “clean checks” gate running custom checks and strict casts.
bun.lock Updates lockfile for new/updated workspace deps and package additions.
biome.json Upgrades useTopLevelRegex from warn → error.
apps/landing/package.json Adds @packrat/guards dependency.
apps/landing/lib/typeAssertions.ts Re-exports assertDefined from @packrat/guards.
apps/landing/lib/icons.tsx Uses isFunction guard for dynamic lucide icon lookup.
apps/guides/scripts/sync-to-r2.ts Documents env cast as safe-cast for R2 sync script.
apps/guides/scripts/enhance-content.ts Hoists timestamp regex; replaces regex-filter with includes-based filter.
apps/guides/package.json Adds @packrat/guards dependency.
apps/guides/lib/assertDefined.ts Re-exports assertDefined from @packrat/guards.
apps/guides/components/search.tsx Documents DOM Node cast as safe-cast.
apps/guides/app/dev/generate/page.tsx Uses assertEnum and improves typing for category iteration.
apps/expo/utils/format-ai-response.ts Hoists formatting regex patterns (mirrors API implementation).
apps/expo/lib/utils/domain-specific-extensions.ts Uses isFunction guard for extension mapping.
apps/expo/lib/utils/dateUtils.ts Uses isString guard for input validation.
apps/expo/lib/store.ts Documents Legend-State proxy typing cast as safe-cast.
apps/expo/features/wildlife/hooks/useWildlifeIdentification.ts Uses isObject guard and documents cast after guard.
apps/expo/features/weather/screens/LocationPreviewScreen.tsx Documents cast from formatted weather data as safe-cast.
apps/expo/features/weather/hooks/useWeatherAlert.ts Documents cast to local subset type as safe-cast.
apps/expo/features/weather/hooks/useLocationSearch.ts Documents cast from formatted weather data as safe-cast.
apps/expo/features/weather/hooks/useLocationRefresh.ts Documents casts for partial updates as safe-cast.
apps/expo/features/trips/screens/TripDetailScreen.tsx Documents cast used before store hydration as safe-cast.
apps/expo/features/trips/components/TripForm.tsx Uses isString and documents form defaultValues cast as safe-cast.
apps/expo/features/trail-conditions/hooks/useTrailConditionReports.ts Documents JSON/treaty/store casts as safe-cast.
apps/expo/features/trail-conditions/components/TrailConditionReportCard.tsx Removes unnecessary enum casts for label lookup.
apps/expo/features/packs/utils/computePackWeights.ts Removes unnecessary WeightUnit cast.
apps/expo/features/packs/store/packs.ts Documents treaty/Zod boundary casts as safe-cast.
apps/expo/features/packs/store/packItems.ts Documents treaty/Zod boundary casts as safe-cast.
apps/expo/features/packs/screens/PackDetailScreen.tsx Documents pack cast rationale as safe-cast.
apps/expo/features/packs/screens/ItemsScanScreen.tsx Documents expo-router params cast as safe-cast.
apps/expo/features/packs/hooks/useSeasonSuggestions.ts Documents treaty response cast as safe-cast.
apps/expo/features/packs/hooks/usePackItemDetailsFromApi.ts Documents Zod→local type cast as safe-cast.
apps/expo/features/packs/hooks/usePackGapAnalysis.ts Documents treaty response cast as safe-cast.
apps/expo/features/packs/hooks/useImageDetection.ts Documents treaty response cast as safe-cast.
apps/expo/features/packs/hooks/useDuplicatePack.ts Documents treaty response cast as safe-cast.
apps/expo/features/packs/components/SimilarItemsForPackItem.tsx Documents list data cast as safe-cast.
apps/expo/features/packs/components/PackCard.tsx Documents Pack/PackInStore cast rationale as safe-cast.
apps/expo/features/packs/components/AddPackItemActions.tsx Uses isFunction guard for ref handling.
apps/expo/features/pack-templates/store/packTemplates.ts Documents treaty/Zod boundary casts as safe-cast.
apps/expo/features/pack-templates/store/packTemplateItems.ts Documents treaty/Zod boundary casts as safe-cast.
apps/expo/features/pack-templates/screens/ItemsScanScreen.tsx Documents expo-router params cast as safe-cast.
apps/expo/features/pack-templates/hooks/usePackTemplateSummary.ts Removes unnecessary WeightUnit casts.
apps/expo/features/pack-templates/hooks/useGenerateTemplateFromOnlineContent.ts Uses isObject guard and documents treaty casts as safe-cast.
apps/expo/features/pack-templates/components/AddPackTemplateItemActions.tsx Uses isFunction guard for ref handling.
apps/expo/features/catalog/screens/CatalogItemsScreen.tsx Documents treaty casts used for list rendering.
apps/expo/features/catalog/hooks/useSimilarItems.ts Documents treaty response cast as safe-cast.
apps/expo/features/catalog/components/SimilarItems.tsx Removes unnecessary cast for list data.
apps/expo/features/catalog/components/CatalogBrowserModal.tsx Documents treaty casts for popular/search/paginated items.
apps/expo/features/auth/hooks/useAuthActions.ts Uses isObject guard and documents treaty/user casts as safe-cast.
apps/expo/features/ai/lib/localModelManager.ts Uses isString guard for platform version parsing.
apps/expo/features/ai/lib/CustomChatTransport.ts Uses isString guard in error formatting.
apps/expo/features/ai/hooks/useReportedContent.ts Documents treaty response cast as safe-cast.
apps/expo/features/ai/hooks/useReportContent.ts Documents treaty response cast as safe-cast.
apps/expo/features/ai/components/ToolInvocationRenderer.tsx Documents tool union narrowing casts as safe-cast.
apps/expo/features/ai/components/ChatBubble.tsx Documents prefix-based narrowing cast to ToolUIPart.
apps/expo/features/ai/components/AIModeSheet.tsx Uses isFunction guard for ref handling.
apps/expo/features/ai/atoms/chatStorageAtoms.ts Uses isObject/isString guards for persisted message validation.
apps/expo/features/ai-packs/hooks/useGeneratedPacks.ts Documents treaty response cast as safe-cast.
apps/expo/components/initial/WeightBadge.tsx Uses isString guard for unit fallback.
apps/expo/components/Icon/get-icon-names.ts Adds safe-cast documentation around icon-name unions.
apps/expo/components/Icon/Icon.tsx Adds safe-cast documentation around icon-name unions.
apps/expo/components/Icon/Icon.ios.tsx Adds safe-cast documentation around icon-name unions.
apps/expo/atoms/atomWithSecureStorage.ts Uses isFunction guard for updater handling.
apps/expo/atoms/atomWithKvStorage.ts Uses isFunction guard for updater handling.
apps/expo/atoms/atomWithAsyncStorage.ts Uses isFunction guard for updater handling.
apps/expo/app/(app)/messages/chat.tsx Uses isString guard for union rendering.
apps/expo/app/(app)/messages/chat.android.tsx Uses isString guard for union rendering.
apps/expo/app/(app)/current-pack/[id].tsx Documents PackItem cast as safe-cast.
apps/expo/app/(app)/(tabs)/profile/index.tsx Uses isString guard for section header rendering.
apps/admin/lib/env.ts Adds Zod-based env shim (used by client code).
apps/admin/lib/api.ts Switches API base URL source to admin env shim; documents fetch-boundary cast.
apps/admin/app/login/page.tsx Uses admin env shim for API base URL.
.github/workflows/checks.yml Adds CI steps for custom linting and strict cast checks.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 72 to 77
if (CF_ACCESS_TEAM_DOMAIN && CF_ACCESS_AUD) {
// CF Access configured: cryptographic JWT verification only, no fallthrough.
const cfIdentity = await verifyCFAccessRequest(request, CF_ACCESS_TEAM_DOMAIN, CF_ACCESS_AUD);
return cfIdentity !== null;
if (cfIdentity !== null) return true;
// CF Access env vars set but no CF JWT — fall back to admin Bearer JWT so that
// Basic-auth login continues to work when CF Access is disabled or bypassed.
}
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adminAuthGuard falls back to Bearer/Basic whenever verifyCFAccessRequest returns null, which happens both when the CF Access header is missing and when it is present but fails verification (bad aud/iss/signature). This weakens CF Access enforcement because a request with an invalid CF JWT can still be authorized via Bearer/Basic even though CF Access is configured. Consider only falling back when the cf-access-jwt-assertion header is absent; if it is present but invalid, return false (or have verifyCFAccessRequest distinguish “missing” vs “invalid”).

Copilot uses AI. Check for mistakes.
Comment thread apps/admin/lib/env.ts
Comment on lines +18 to +21
* Typed env parsed from `process.env` at module load. Throws a Zod
* validation error if any value fails its schema constraint.
*/
export const adminEnv = adminEnvSchema.parse(process.env);
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adminEnvSchema.parse(process.env) is imported from client components (e.g. app/login/page.tsx). In Next.js client bundles, process.env as an object is typically {} at runtime and only direct process.env.NEXT_PUBLIC_* property reads are inlined, so this parse is likely to throw in the browser. Prefer parsing an explicit object (e.g. { NEXT_PUBLIC_API_URL: process.env.NEXT_PUBLIC_API_URL }) or reusing the existing @packrat/env/next client-safe env shim.

Suggested change
* Typed env parsed from `process.env` at module load. Throws a Zod
* validation error if any value fails its schema constraint.
*/
export const adminEnv = adminEnvSchema.parse(process.env);
* Typed env parsed from explicit `process.env.NEXT_PUBLIC_*` reads at
* module load. Throws a Zod validation error if any value fails its
* schema constraint.
*/
export const adminEnv = adminEnvSchema.parse({
NEXT_PUBLIC_API_URL: process.env.NEXT_PUBLIC_API_URL,
});

Copilot uses AI. Check for mistakes.
Comment on lines 123 to 135
function shouldSkipLine(line: string): boolean {
const trimmed = line.trimStart();
return (
trimmed.startsWith('import ') ||
trimmed.startsWith('export ') ||
trimmed.startsWith('//') ||
trimmed.startsWith('*') ||
trimmed.startsWith('export {') ||
trimmed.startsWith('export type {') ||
trimmed.startsWith('// ') ||
trimmed.startsWith('//\t') ||
trimmed === '//' ||
trimmed.startsWith('* ') ||
trimmed === '*' ||
trimmed.startsWith('/*')
);
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldSkipLine only skips comment lines that start with // or //\t (or exactly //), which means comments like //TODO, ////, or //eslint-disable will be scanned and any quoted text inside them may be counted as a “magic string”. This is likely to create noisy/incorrect results and contradicts the header comment that says comment lines are excluded. Consider skipping all // comment lines (and * block-comment lines) regardless of the following character.

Copilot uses AI. Check for mistakes.
Comment on lines +54 to +55
- name: Check for unauthenticated routes
run: bun scripts/lint/no-unauth-routes.ts
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Custom lint rules step runs bun lint:custom, and lint:custom already includes scripts/lint/no-unauth-routes.ts. The following Check for unauthenticated routes step duplicates that work and can drift if the script list changes. Consider removing the extra step or narrowing lint:custom so each check runs exactly once in CI.

Suggested change
- name: Check for unauthenticated routes
run: bun scripts/lint/no-unauth-routes.ts

Copilot uses AI. Check for mistakes.
Comment thread package.json
Comment on lines 14 to +37
@@ -33,7 +34,7 @@
"ios": "cd apps/expo && bun ios",
"lefthook": "lefthook install",
"lint": "biome check --write",
"lint:custom": "bun run scripts/lint/no-raw-typeof.ts && bun run scripts/lint/no-raw-regex.ts && bun run packages/env/scripts/no-raw-process-env.ts",
"lint:custom": "bun run scripts/lint/no-raw-typeof.ts && bun run scripts/lint/no-raw-regex.ts && bun run packages/env/scripts/no-raw-process-env.ts && bun run scripts/lint/no-duplicate-guards.ts && bun run scripts/lint/no-unauth-routes.ts",
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR scope: this change set introduces substantial additional functionality and tooling (e.g. new check:all, new custom lint scripts, and new API routes/packages) beyond the admin CF Access fallback described in the PR title/description. Consider splitting these into separate PRs or updating the PR description to reflect the broader changes to reduce review/rollback risk.

Copilot uses AI. Check for mistakes.
…o CF JWT present

When CF_ACCESS_TEAM_DOMAIN/CF_ACCESS_AUD are configured but a request arrives
without a CF Access JWT (e.g. after disabling CF Access on the dashboard while
env vars are still set), the guard previously rejected the request outright.
Admin Bearer JWTs issued by /token are now accepted as a fallback so Basic-auth
login continues to work during transitions.
@andrew-bierman andrew-bierman force-pushed the fix/admin-basic-auth-fallback branch from 43e613c to 646e485 Compare April 27, 2026 00:26
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

Note

Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
apps/expo/features/ai/lib/localModelManager.ts (1)

83-95: ⚠️ Potential issue | 🔴 Critical

isAppleIntelligenceAvailable() accesses a Promise as if it were a module object.

getAppleModule() returns an unawaited dynamic import() Promise (line 61), but isAppleIntelligenceAvailable() treats mod as the resolved module at line 94. Accessing mod.apple on a Promise fails, causing the function to always return false and block Apple Intelligence activation.

The fix requires making getAppleModule() and isAppleIntelligiceAvailable() async and awaiting the Promise. All call sites (AIModeSheet, settings, getLocalModel, initializeModel, downloadLocalModel) must then await the function.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/expo/features/ai/lib/localModelManager.ts` around lines 83 - 95,
isAppleIntelligenceAvailable() treats the result of getAppleModule() as an
already-resolved module but getAppleModule() returns a Promise from dynamic
import; change getAppleModule() and isAppleIntelligenceAvailable() to be async
and await the import (e.g., await getAppleModule()) so you access the actual
module (mod.apple) instead of a Promise, update the try/catch to await
mod.apple.isAvailable() if that method is async, and then update every caller
(AIModeSheet, settings, getLocalModel, initializeModel, downloadLocalModel) to
await isAppleIntelligenceAvailable() and handle the returned Promise
accordingly.
packages/api/src/routes/catalog/index.ts (1)

313-313: ⚠️ Potential issue | 🟡 Minor

Move numeric id parameter validation to Zod schema layer to prevent NaN errors.

The routes at lines 313, 349, 407, and 468 convert params.id to a number without schema-level validation. When params.id is non-numeric, Number(params.id) produces NaN, which causes a SQL error (invalid input syntax for type integer: "NaN") when passed to Drizzle. Pre-existing issue noted by CI failures, but consider fixing all four routes by validating in the Zod schema instead:

♻️ Suggested params schema
-      params: z.object({ id: z.string() }),
+      params: z.object({ id: z.coerce.number().int().positive() }),

(then drop the inline Number(params.id) cast)

The admin routes handle this with post-conversion checks (if (!Number.isFinite(id) || id <= 0)); applying Zod coercion here is cleaner and consistent.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/api/src/routes/catalog/index.ts` at line 313, Update the route
params Zod schema to coerce and validate the id as a positive integer (e.g., use
z.coerce.number().int().positive() or equivalent) and use that parsed value in
handlers; remove the inline Number(params.id) casts (references: the handlers
that assign itemId / id at the locations where you currently call
Number(params.id) — see the variable itemId) so the route receives a validated
numeric id from the schema, preventing NaN reaching Drizzle; ensure all four
routes (the ones currently converting params.id) use the shared/coerced params
schema and drop post-conversion finite checks if Zod guarantees positivity and
integrality.
packages/api/src/routes/packs/index.ts (1)

173-177: ⚠️ Potential issue | 🔴 Critical

Critical: Handle undefined image before calling .startsWith().

The pipeline failure indicates a TypeError when image is undefined. The code destructures image from body and immediately calls .startsWith() without checking if it exists. If the Zod schema allows image to be undefined or if validation is bypassed, this will throw a runtime error.

🐛 Proposed fix to add undefined check
     try {
       const { image, matchLimit } = body;
 
-      if (!image.startsWith(`${user.userId}-`)) {
+      if (!image || !image.startsWith(`${user.userId}-`)) {
         return status(403, { error: 'Unauthorized' });
       }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/api/src/routes/packs/index.ts` around lines 173 - 177, Check for a
missing or non-string image before calling .startsWith: ensure the destructured
image from body is defined and typeof image === 'string' first; if image is
missing/invalid return a 400 Bad Request (e.g., status(400, { error: 'Missing
image' })), otherwise keep the existing ownership check and return 403
Unauthorized when image is a string but does not start with `${user.userId}-`;
reference the variables image, body, status and user.userId in the patch.
packages/api/src/routes/alltrails.ts (1)

38-79: ⚠️ Potential issue | 🟡 Minor

Auth + SSRF guard are correct; single-redirect limitation is a potential issue.

The redirect: 'manual' + hostname/protocol re-validation is the right approach, and isAuthenticated: true properly closes the public preview route.

The post-validation fetch on line 66 uses redirect: 'error', permitting only a single redirect hop. Real-world testing confirms AllTrails uses redirects (e.g., alltrails.comwww.alltrails.com, and canonical paths with i18n), but sampled URLs show at most 2-hop chains. However, if users encounter URLs with 3+ hop chains, the second fetch will fail with a 502 error. To future-proof this, consider switching the second fetch to redirect: 'manual' and looping the same validate-then-fetch logic with a small bounded hop count (e.g., 3) rather than erroring on subsequent hops.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/api/src/routes/alltrails.ts` around lines 38 - 79, The current
post-validation fetch uses redirect: 'error' which fails on multi-hop redirects;
change the logic to perform the second fetch with redirect: 'manual' and
implement a bounded loop (e.g., maxHops = 3) that repeats: fetch (headers: {
'User-Agent': UA }, redirect: 'manual', signal: AbortSignal.timeout(8000)),
check for 3xx and validate location (new URL(location, currentUrl)) against
protocol === 'https:' and ALLTRAILS_HOSTNAME_RE, update currentUrl to the
validated redirectUrl and continue, otherwise break and handle non-ok responses
the same way; keep existing timeout/DOMException handling and same status(...)
error returns, and ensure the loop exits with a 502 if hop limit exceeded.
🟡 Minor comments (14)
packages/analytics/src/core/local-cache.ts-165-165 (1)

165-165: ⚠️ Potential issue | 🟡 Minor

Keep Line 165 within Biome width limit.

The inline cast note is too long for the 100-character rule. Move it to a short block comment above the return (or trim it) to avoid formatter/lint churn.

Suggested tweak
-      return obj as T; // safe-cast: caller-provided generic boundary — obj is built from DuckDB column names and the caller's return type annotation verifies the shape
+      // Safe cast at query boundary: object keys come from DuckDB column names.
+      return obj as T;

As per coding guidelines, "**/*.{js,jsx,ts,tsx,json}: Enforce 2 spaces indentation, 100 character line width, and single quotes via Biome 2.0".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/analytics/src/core/local-cache.ts` at line 165, The inline cast
comment on the "return obj as T" statement exceeds the 100-char Biome width;
move the explanatory note into a short block comment immediately above the
return (or shorten it to under 100 chars) so the return line stays within width
limits while preserving the explanation that the cast is safe because the object
is built from DuckDB column names and the caller's generic verifies the shape.
apps/expo/components/Icon/get-icon-names.ts-32-33 (1)

32-33: ⚠️ Potential issue | 🟡 Minor

Tone down “safe-cast” wording where runtime validity is assumed, not proven.

These comments currently read as guaranteed validity, but name is still a plain string input in these paths. Reword to “assumed by contract/fallback behavior” to avoid implying enforced runtime validation.

Proposed wording adjustment
-// safe-cast: caller passes name under sfSymbol naming scheme; SfSymbolName is a string
-// union from expo-symbols and cannot be checked without a full lookup table.
+// cast by contract: `name` is expected to be an SF Symbol string when namingScheme is 'sfSymbol'.
+// This path does not fully validate membership against the complete SF Symbol union at runtime.
-// safe-cast: no mapping found; assume name is a MaterialCommunityIcons name as fallback
+// fallback cast: when no mapping is found, we treat `name` as a MaterialCommunityIcons name by convention.
+// This is an assumption-based fallback, not a strict runtime membership check.

Also applies to: 44-45, 53-54, 93-94

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/expo/components/Icon/get-icon-names.ts` around lines 32 - 33, Replace
the "safe-cast" phrasing in the comments that claim runtime validity with
wording that indicates the value is assumed by contract or will fall back if
invalid; specifically edit the comments near the uses of the string input
variable name and the SfSymbolName reference in get-icon-names.ts (the comment
blocks around the occurrences at the earlier checks — e.g., where the code
mentions "safe-cast: caller passes name under sfSymbol naming scheme;
SfSymbolName is a string union from expo-symbols and cannot be checked without a
full lookup table") to something like "assumed by contract / fallback behavior:
caller is expected to supply an sfSymbol-style name; name remains a plain string
at runtime and is not exhaustively validated here." Ensure you update the same
wording at the other occurrences noted (around lines 44-45, 53-54, and 93-94).
apps/guides/scripts/sync-to-r2.ts-41-44 (1)

41-44: ⚠️ Potential issue | 🟡 Minor

Trim duplicated cast comments and keep within line-width limit.

The “safe-cast” explanation is duplicated (Lines 41-42 and inline on Line 44), and the inline comment is very long.
Keep a single concise comment to satisfy readability/lint constraints.

As per coding guidelines, "**/*.{js,jsx,ts,tsx,json}: Enforce 2 spaces indentation, 100 character line width, and single quotes via Biome 2.0".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/guides/scripts/sync-to-r2.ts` around lines 41 - 44, Remove the
duplicated "safe-cast" explanation and replace it with one concise, single-line
comment that fits the 100-char line width, e.g. above the R2BucketService
instantiation; keep the cast intact (env as unknown as ValidatedEnv) and
reference the relevant types in the short comment (SyncEnv carries the S3/R2
credentials R2BucketService needs). Ensure the comment uses single quotes and
the file's existing 2-space indentation.
apps/expo/features/pack-templates/hooks/useGenerateTemplateFromOnlineContent.ts-67-73 (1)

67-73: ⚠️ Potential issue | 🟡 Minor

Prevent "[object Object]" error messages in fallback path.

When value is an object without error, message becomes that object and String(message) turns into "[object Object]".
Use a string-only fallback and type-check extracted fields.

Proposed fix
-        const message = isObject(value) && value?.error ? value.error : (value ?? 'Import failed');
+        const message =
+          isObject(value) && typeof value.error === 'string'
+            ? value.error
+            : typeof value === 'string'
+              ? value
+              : 'Import failed';
         // safe-cast: augmenting the base Error with ImportError fields assigned immediately below
         const importError = new Error(String(message)) as ImportError;
         importError.status = error.status;
         if (isObject(value)) {
-          importError.code = value.code;
-          importError.existingTemplateId = value.existingTemplateId;
+          if (typeof value.code === 'string') importError.code = value.code;
+          if (typeof value.existingTemplateId === 'string') {
+            importError.existingTemplateId = value.existingTemplateId;
+          }
         }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/expo/features/pack-templates/hooks/useGenerateTemplateFromOnlineContent.ts`
around lines 67 - 73, The current fallback builds message using String(value)
which yields "[object Object]" when value is an object; change the logic in
useGenerateTemplateFromOnlineContent.ts so message is always a string (e.g., use
typeof checks: if isObject(value) && typeof value.error === 'string' use
value.error, else if typeof value === 'string' use value, otherwise use a
literal 'Import failed'), then create importError from that string; also guard
assignments to importError.code and importError.existingTemplateId by
type-checking value.code and value.existingTemplateId (only assign if they have
the expected types) while still setting importError.status = error.status.
packages/checks/src/check-magic-strings.ts-123-136 (1)

123-136: ⚠️ Potential issue | 🟡 Minor

shouldSkipLine misses a few common comment/import shapes — minor noise risk.

Heuristic gaps that can let literals through:

  • Closing line of a multi-line import/export (e.g. } from '@foo/bar';) — usually neutralized by the @/relative-path/slash filters in shouldIgnoreLiteral, but not for bare-name specifiers.
  • JSDoc/block-comment lines whose content doesn't begin with * (rare but valid).
  • Trailing inline block comments on real code lines (e.g. foo(); /* see "magic" */).

Given the aggressive shouldIgnoreLiteral filters, the practical false-positive rate is low — flagging only so the assumptions are explicit. If you want to harden this without a real parser, a single pass that strips block comments before line iteration would close most of the gap.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/checks/src/check-magic-strings.ts` around lines 123 - 136, The
current shouldSkipLine heuristic misses closing import/export lines, some
JSDoc/block-comment shapes and trailing inline block comments; to fix, remove
block comments before per-line analysis (so shouldSkipLine only sees code or
line comments) by running a single-pass strip of /* ... */ (including multiline)
from the source text prior to splitting into lines, and then keep shouldSkipLine
for line-level checks (function shouldSkipLine) while leaving literal filtering
in shouldIgnoreLiteral intact; alternatively, if you prefer not to strip at
source level, extend shouldSkipLine to also skip lines that match closing
import/export patterns (e.g., lines that start with '}' or that end with
/from\s+['"].+['"];?/), JSDoc opening/closing markers, and lines containing only
inline block comments, but the preferred change is the single-pass block-comment
removal before invoking shouldSkipLine.
apps/admin/lib/env.ts-11-13 (1)

11-13: ⚠️ Potential issue | 🟡 Minor

Use Zod 4's standalone z.url() instead of z.string().url().

z.string().url() is deprecated in Zod 4 and will be removed in a future major version. The string format validators have been promoted to top-level functions on the z namespace. Replace with z.url() to avoid deprecation warnings and prepare for future Zod versions.

♻️ Refactor
 const adminEnvSchema = z.object({
-  NEXT_PUBLIC_API_URL: z.string().url(),
+  NEXT_PUBLIC_API_URL: z.url(),
 });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/admin/lib/env.ts` around lines 11 - 13, The adminEnvSchema currently
uses the deprecated z.string().url() for NEXT_PUBLIC_API_URL; update
adminEnvSchema to use the Zod 4 standalone validator z.url() for the
NEXT_PUBLIC_API_URL field (replace z.string().url() with z.url()) so it no
longer relies on the deprecated string format API and avoids warnings.
apps/expo/features/catalog/screens/CatalogItemsScreen.tsx-70-77 (1)

70-77: ⚠️ Potential issue | 🟡 Minor

Remove dead code filter and validate vector search response with Zod schema.

The as unknown as CatalogItem[] double-cast on line 71 and single as CatalogItem[] on line 76 are correctly differentiated: the vector search response includes { similarity: number } (hence the double cast), while paginated items are Zod-validated by CatalogItemsResponseSchema.parse() (hence the single cast). Both cast patterns are justified.

However, the .filter((item) => Boolean(item?.id)) on line 77 is dead code—CatalogItemSchema defines id as required (z.number().int().positive()), so Zod validation already guarantees no items lack an id.

For consistency with the paginated response, consider adding a Zod schema for the vector search response (currently unvalidated Treaty-inferred type) instead of relying on raw type assertions.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/expo/features/catalog/screens/CatalogItemsScreen.tsx` around lines 70 -
77, The paginatedItems.filter(Boolean(item?.id)) is dead since
CatalogItemsResponseSchema.parse() already guarantees id; remove that .filter
call from paginatedItems. Also replace the unchecked double-cast for
searchResults by defining and using a Zod schema for the vector search shape
(e.g., VectorSearchItemSchema / VectorSearchResponseSchema) and call its
.parse() on vectorResult.items before assigning to searchResults so you no
longer need the as unknown as CatalogItem[] assertion; reference vectorResult,
searchResults, paginatedItems, CatalogItemSchema and CatalogItemsResponseSchema
when making these changes.
packages/api/src/services/embeddingService.ts-7-7 (1)

7-7: ⚠️ Potential issue | 🟡 Minor

Handle CRLF in embedding normalization too.

/\n/g misses Windows-style newlines and can leave \r artifacts in input. Use \r?\n for consistent cleaning.

Proposed tweak
-const NEWLINE = /\n/g;
+const NEWLINE = /\r?\n/g;

Also applies to: 34-34, 54-54

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/api/src/services/embeddingService.ts` at line 7, Update the newline
regex to handle Windows CRLFs by changing the NEWLINE constant from /\n/g to
/\r?\n/g and replace any other raw occurrences of /\n/g in this file (e.g.,
places that normalize embedding input using NEWLINE) so CR characters are not
left behind; ensure all embedding-normalization code paths use the NEWLINE
symbol (or the new /\r?\n/g) so inputs are consistently cleaned of CRLF
artifacts.
apps/guides/scripts/enhance-content.ts-112-115 (1)

112-115: ⚠️ Potential issue | 🟡 Minor

Pattern semantics changed from regex to substring — update help text and examples.

getContentFiles now treats --pattern as a case-insensitive substring (includes), not a regex. The CLI help still calls it <regex> and the example --pattern "hiking|backpacking" only matches filenames containing the literal hiking|backpacking, which won't match anything realistic. Either update the docs (preferred for this minor refactor) or restore regex behavior.

📝 Proposed help/example fix
-  -p, --pattern <regex>  Only process files matching pattern (legacy mode)
+  -p, --pattern <substring>  Only process files whose name contains this case-insensitive substring (legacy mode)
@@
   # Pattern-based processing
-  bun run enhance-content --pattern "hiking|backpacking" --max-files 5
+  bun run enhance-content --pattern hiking --max-files 5
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/guides/scripts/enhance-content.ts` around lines 112 - 115, Update the
CLI help and examples to reflect that getContentFiles now treats --pattern as a
case-insensitive substring match (uses
path.basename(...).toLowerCase().includes) rather than a regex; change the help
label from "<regex>" to "<pattern>" or "<substring>" and replace examples like
--pattern "hiking|backpacking" with realistic substring examples such as
--pattern "hiking" or --pattern "backpacking" so the documented behavior matches
the implementation in getContentFiles/files.filter.
scripts/lint/no-unauth-routes.ts-96-117 (1)

96-117: ⚠️ Potential issue | 🟡 Minor

Naive paren-depth scan can miscompute callEnd.

The forward walk on Lines 106–116 counts every ( / ) regardless of context, so parens appearing inside string literals, template literals, regex literals, or comments within the route handler will throw off depth and either truncate the span early (causing a false positive when the auth macro lies past the truncated end) or extend it past the real call (rare). Examples that would trip it:

.post('/x', async ({ body }) => {
  if (s === ')') return;          // ← unbalanced ')' in string literal
  const re = /[()]/g;             // ← parens in regex
  // Don't drop tables :) (kidding)
}, { isAuthenticated: true })

For a lint-only heuristic this may be acceptable, but worth noting in a comment that the scanner is line/char-naive and may need replacing with a proper TS parser (e.g., @typescript-eslint/parser) if false positives appear.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/lint/no-unauth-routes.ts` around lines 96 - 117, The paren-depth scan
that uses ROUTE_START, parenOpen, depth, and callEnd is naive and will be thrown
off by parentheses inside string literals, template literals, regexes, or
comments; update the code by adding a clear comment immediately above the loop
that walks from parenOpen to callEnd describing this limitation (mentioning
strings, template literals, regexes, and comments as examples) and recommend
replacing this heuristic with a real AST-based parser (e.g.,
`@typescript-eslint/parser`) if false positives/negatives become an issue;
alternatively, if you prefer to keep the heuristic, note that any future fix
should implement proper token-aware scanning to skip string/regex/comment tokens
when computing depth.
packages/overpass/tsconfig.json-1-10 (1)

1-10: ⚠️ Potential issue | 🟡 Minor

Add noUncheckedIndexedAccess or extend the root tsconfig.json.

strict: true enables strictNullChecks, but noUncheckedIndexedAccess is a separate flag and isn't covered by strict. The root tsconfig.json already defines both of these settings. For consistency with other packages in the monorepo (e.g., packages/api-client, apps/expo), this config should extend the root:

🔧 Proposed fix
 {
-  "compilerOptions": {
-    "target": "ES2022",
-    "module": "ESNext",
-    "moduleResolution": "bundler",
-    "strict": true,
-    "skipLibCheck": true
-  },
-  "include": ["src/**/*.ts"]
+  "extends": "../../tsconfig.json",
+  "compilerOptions": {
+    "skipLibCheck": true
+  },
+  "include": ["src/**/*.ts"]
 }

If extending the root config is not desired, add noUncheckedIndexedAccess: true to the compilerOptions to satisfy the repo's TypeScript strictness policy.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/overpass/tsconfig.json` around lines 1 - 10, Update this package's
tsconfig.json to match the monorepo's strictness: either add
"noUncheckedIndexedAccess": true under compilerOptions, or have the tsconfig
extend the workspace root tsconfig so it inherits noUncheckedIndexedAccess and
other shared flags; modify the tsconfig.json's "compilerOptions" (or add an
"extends" entry) to ensure noUncheckedIndexedAccess is enabled consistently with
other packages.
packages/mcp/src/tools/trails.ts-69-69 (1)

69-69: ⚠️ Potential issue | 🟡 Minor

Constrain osmId to a numeric string to harden the URL interpolation.

osmId is interpolated directly into /trails/${osmId} and /trails/${osmId}/geometry (Lines 74 and 94). With only z.string(), an LLM could pass values like 123/../admin, .., or a URL-encoded segment, which would alter the resolved path or hit unintended routes. The example in the description ("1243746") implies a numeric OSM relation ID; tightening the schema closes that gap regardless of server-side validation:

🛡️ Suggested change
       inputSchema: {
-        osmId: z.string().describe('OSM relation ID of the trail (e.g. "1243746")'),
+        osmId: z
+          .string()
+          .regex(/^\d+$/)
+          .describe('OSM relation ID of the trail (e.g. "1243746")'),
       },

Also applies to: 89-89

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/mcp/src/tools/trails.ts` at line 69, The osmId zod schema currently
uses z.string() which allows non-numeric values to be interpolated into routes
like /trails/${osmId} and /trails/${osmId}/geometry; change the osmId definition
(the osmId field in the Zod schema in packages/mcp/src/tools/trails.ts) to
constrain it to a numeric string (e.g. z.string().regex(/^\d+$/) or
z.string().refine(s => /^\d+$/.test(s))) and update the description/example
accordingly so only digits are accepted; apply the same stricter schema wherever
osmId is defined/used for those endpoints.
packages/api/src/routes/trails/index.ts-74-79 (1)

74-79: ⚠️ Potential issue | 🟡 Minor

Number(params.osmId) drops precision past 2^53; pass the validated string straight to the builder.

TrailQueryBuilder.id() already accepts number | string | bigint, and Overpass treats the ID as an opaque integer literal — round-tripping through Number adds nothing except a precision ceiling. OSM relation IDs are currently well below Number.MAX_SAFE_INTEGER, but ways and nodes already exceed it in some regions, and osmId is shared terminology. Validate as a digit string and forward it.

This change applies identically to /:osmId (Lines 74-79) and /:osmId/geometry (Lines 110-115).

🛡️ Proposed fix
-      const osmId = Number(params.osmId);
-      if (!Number.isInteger(osmId) || osmId <= 0) {
-        return status(400, { error: 'osmId must be a positive integer' });
-      }
-
-      const ql = new TrailQueryBuilder().id(osmId).build();
+      if (!/^[1-9]\d*$/.test(params.osmId)) {
+        return status(400, { error: 'osmId must be a positive integer' });
+      }
+
+      const ql = new TrailQueryBuilder().id(params.osmId).build();

(Pair with the id() validation suggestion in builder.ts so untrusted strings can't reach the QL output.)

Also applies to: 110-115

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/api/src/routes/trails/index.ts` around lines 74 - 79, Replace the
numeric coercion of params.osmId with digit-string validation and pass the
original string into TrailQueryBuilder.id instead of calling Number(...);
specifically, remove Number(params.osmId), validate that params.osmId matches
/^\d+$/ and return status(400, ...) if not a positive digit string, then call
new TrailQueryBuilder().id(params.osmId).build(); apply this change in both
route handlers that use params.osmId (the handler that builds the query and the
'/:osmId/geometry' handler) so the id() receives the validated string and avoids
Number precision loss.
scripts/lint/no-duplicate-guards.ts-78-79 (1)

78-79: ⚠️ Potential issue | 🟡 Minor

IMPL_PATTERN misses async function declarations.

The regex requires function\s+ immediately after the optional export\s+, so export async function isString(...) and async function isString(...) slip past the scan. Unlikely for sync guards like isString, but realistic for things like assertError or any future async narrower added to the guard list.

🛠 Proposed fix
-const IMPL_PATTERN =
-  /(?:export\s+)?(?:function\s+|const\s+|let\s+)([A-Za-z][A-Za-z0-9_]*)\s*(?:[=(:<])/g;
+const IMPL_PATTERN =
+  /(?:export\s+)?(?:async\s+)?(?:function\s+|const\s+|let\s+|var\s+)([A-Za-z][A-Za-z0-9_]*)\s*(?:[=(:<])/g;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/lint/no-duplicate-guards.ts` around lines 78 - 79, The IMPL_PATTERN
regex misses declarations with an optional async keyword (e.g., "async function
foo" or "export async function foo"); update the IMPL_PATTERN to allow an
optional "async" token between the optional export and the "function" part (for
example by adding an optional non-capturing group like (?:async\s+)? before the
function alternative), keeping the existing capture group for the identifier and
preserving other branches (const/let and function forms) so both async function
and non-async declarations are matched.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 533cc89e-057f-484d-9c94-b61cda31fc88

📥 Commits

Reviewing files that changed from the base of the PR and between 1dfb8b1 and 43e613c.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (145)
  • .github/workflows/checks.yml
  • apps/admin/app/login/page.tsx
  • apps/admin/lib/api.ts
  • apps/admin/lib/env.ts
  • apps/expo/app/(app)/(tabs)/profile/index.tsx
  • apps/expo/app/(app)/current-pack/[id].tsx
  • apps/expo/app/(app)/messages/chat.android.tsx
  • apps/expo/app/(app)/messages/chat.tsx
  • apps/expo/atoms/atomWithAsyncStorage.ts
  • apps/expo/atoms/atomWithKvStorage.ts
  • apps/expo/atoms/atomWithSecureStorage.ts
  • apps/expo/components/Icon/Icon.ios.tsx
  • apps/expo/components/Icon/Icon.tsx
  • apps/expo/components/Icon/get-icon-names.ts
  • apps/expo/components/initial/WeightBadge.tsx
  • apps/expo/features/ai-packs/hooks/useGeneratedPacks.ts
  • apps/expo/features/ai/atoms/chatStorageAtoms.ts
  • apps/expo/features/ai/components/AIModeSheet.tsx
  • apps/expo/features/ai/components/ChatBubble.tsx
  • apps/expo/features/ai/components/ToolInvocationRenderer.tsx
  • apps/expo/features/ai/hooks/useReportContent.ts
  • apps/expo/features/ai/hooks/useReportedContent.ts
  • apps/expo/features/ai/lib/CustomChatTransport.ts
  • apps/expo/features/ai/lib/localModelManager.ts
  • apps/expo/features/auth/hooks/useAuthActions.ts
  • apps/expo/features/catalog/components/CatalogBrowserModal.tsx
  • apps/expo/features/catalog/components/SimilarItems.tsx
  • apps/expo/features/catalog/hooks/useSimilarItems.ts
  • apps/expo/features/catalog/screens/CatalogItemsScreen.tsx
  • apps/expo/features/pack-templates/components/AddPackTemplateItemActions.tsx
  • apps/expo/features/pack-templates/hooks/useGenerateTemplateFromOnlineContent.ts
  • apps/expo/features/pack-templates/hooks/usePackTemplateSummary.ts
  • apps/expo/features/pack-templates/screens/ItemsScanScreen.tsx
  • apps/expo/features/pack-templates/store/packTemplateItems.ts
  • apps/expo/features/pack-templates/store/packTemplates.ts
  • apps/expo/features/packs/components/AddPackItemActions.tsx
  • apps/expo/features/packs/components/PackCard.tsx
  • apps/expo/features/packs/components/SimilarItemsForPackItem.tsx
  • apps/expo/features/packs/hooks/useDuplicatePack.ts
  • apps/expo/features/packs/hooks/useImageDetection.ts
  • apps/expo/features/packs/hooks/usePackGapAnalysis.ts
  • apps/expo/features/packs/hooks/usePackItemDetailsFromApi.ts
  • apps/expo/features/packs/hooks/useSeasonSuggestions.ts
  • apps/expo/features/packs/screens/ItemsScanScreen.tsx
  • apps/expo/features/packs/screens/PackDetailScreen.tsx
  • apps/expo/features/packs/store/packItems.ts
  • apps/expo/features/packs/store/packs.ts
  • apps/expo/features/packs/utils/computePackWeights.ts
  • apps/expo/features/trail-conditions/components/TrailConditionReportCard.tsx
  • apps/expo/features/trail-conditions/hooks/useTrailConditionReports.ts
  • apps/expo/features/trips/components/TripForm.tsx
  • apps/expo/features/trips/screens/TripDetailScreen.tsx
  • apps/expo/features/weather/hooks/useLocationRefresh.ts
  • apps/expo/features/weather/hooks/useLocationSearch.ts
  • apps/expo/features/weather/hooks/useWeatherAlert.ts
  • apps/expo/features/weather/screens/LocationPreviewScreen.tsx
  • apps/expo/features/wildlife/hooks/useWildlifeIdentification.ts
  • apps/expo/lib/store.ts
  • apps/expo/lib/utils/dateUtils.ts
  • apps/expo/lib/utils/domain-specific-extensions.ts
  • apps/expo/utils/format-ai-response.ts
  • apps/guides/app/dev/generate/page.tsx
  • apps/guides/components/search.tsx
  • apps/guides/lib/assertDefined.ts
  • apps/guides/package.json
  • apps/guides/scripts/enhance-content.ts
  • apps/guides/scripts/sync-to-r2.ts
  • apps/landing/lib/icons.tsx
  • apps/landing/lib/typeAssertions.ts
  • apps/landing/package.json
  • biome.json
  • lefthook.yml
  • package.json
  • packages/analytics/package.json
  • packages/analytics/src/core/data-export.ts
  • packages/analytics/src/core/enrichment.ts
  • packages/analytics/src/core/entity-resolver.ts
  • packages/analytics/src/core/local-cache.ts
  • packages/analytics/src/core/spec-parser.ts
  • packages/analytics/src/core/type-assertions.ts
  • packages/api-client/package.json
  • packages/api-client/src/index.ts
  • packages/api/package.json
  • packages/api/src/containers/AppContainer.ts
  • packages/api/src/index.ts
  • packages/api/src/middleware/auth.ts
  • packages/api/src/routes/admin/index.ts
  • packages/api/src/routes/ai/index.ts
  • packages/api/src/routes/alltrails.ts
  • packages/api/src/routes/auth/index.ts
  • packages/api/src/routes/catalog/index.ts
  • packages/api/src/routes/index.ts
  • packages/api/src/routes/knowledgeBase/reader.ts
  • packages/api/src/routes/packTemplates/index.ts
  • packages/api/src/routes/packs/index.ts
  • packages/api/src/routes/trailConditions/reports.ts
  • packages/api/src/routes/trails/index.ts
  • packages/api/src/routes/weather.ts
  • packages/api/src/routes/wildlife/index.ts
  • packages/api/src/services/aiService.ts
  • packages/api/src/services/embeddingService.ts
  • packages/api/src/services/etl/CatalogItemValidator.ts
  • packages/api/src/services/etl/processValidItemsBatch.ts
  • packages/api/src/services/executeSqlAiTool.ts
  • packages/api/src/services/r2-bucket.ts
  • packages/api/src/services/refreshTokenService.ts
  • packages/api/src/utils/auth.ts
  • packages/api/src/utils/compute-pack.ts
  • packages/api/src/utils/csv-utils.ts
  • packages/api/src/utils/env-validation.ts
  • packages/api/src/utils/format-ai-response.ts
  • packages/api/test/setup.ts
  • packages/checks/src/check-magic-strings.ts
  • packages/checks/src/check-type-casts.ts
  • packages/cli/package.json
  • packages/cli/src/args.ts
  • packages/cli/src/shared.ts
  • packages/config/package.json
  • packages/config/src/config.ts
  • packages/env/scripts/no-raw-process-env.ts
  • packages/guards/src/narrow.ts
  • packages/mcp/src/tools/packs.ts
  • packages/mcp/src/tools/trail-conditions.ts
  • packages/mcp/src/tools/trails.ts
  • packages/mcp/src/tools/trips.ts
  • packages/overpass/package.json
  • packages/overpass/src/builder.test.ts
  • packages/overpass/src/builder.ts
  • packages/overpass/src/client.ts
  • packages/overpass/src/index.ts
  • packages/overpass/src/schemas.ts
  • packages/overpass/src/transform.test.ts
  • packages/overpass/src/transform.ts
  • packages/overpass/src/types.ts
  • packages/overpass/tsconfig.json
  • packages/overpass/vitest.config.ts
  • packages/web-ui/src/components/chart.tsx
  • packages/web-ui/src/components/form.tsx
  • packages/web-ui/src/components/sidebar.tsx
  • packages/web-ui/src/lib/utils.ts
  • scripts/check-all.ts
  • scripts/lint/no-duplicate-guards.ts
  • scripts/lint/no-raw-regex.ts
  • scripts/lint/no-raw-typeof.ts
  • scripts/lint/no-unauth-routes.ts

Comment thread apps/admin/lib/env.ts
* Typed env parsed from `process.env` at module load. Throws a Zod
* validation error if any value fails its schema constraint.
*/
export const adminEnv = adminEnvSchema.parse(process.env);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# 1) Find every site that reads process.env in the admin app (should only be env.ts after the fix).
rg -nP --type=ts --type=tsx -C2 '\bprocess\.env\b' apps/admin

# 2) Confirm adminEnv is only consumed via property access (safe) post-fix.
rg -nP --type=ts --type=tsx -C2 '\badminEnv\b' apps/admin

Repository: PackRat-AI/PackRat

Length of output: 118


🏁 Script executed:

# Fixed ripgrep syntax - use regex patterns without tsx type
rg --type=ts -C2 'process\.env' apps/admin

Repository: PackRat-AI/PackRat

Length of output: 691


🏁 Script executed:

# Also search for adminEnv usage
rg --type=ts -C2 'adminEnv' apps/admin

Repository: PackRat-AI/PackRat

Length of output: 2039


🌐 Web query:

Next.js DefinePlugin process.env inlining behavior client bundle

💡 Result:

In Next.js, the Webpack DefinePlugin is used internally to inline process.env variables into client bundles at build time, specifically for variables prefixed with NEXT_PUBLIC_ or defined in next.config.js env. This replaces exact string matches like process.env.NEXT_PUBLIC_FOO with their literal values (e.g., '"bar"'), enabling dead code elimination and reducing bundle size. Non-public env vars are not inlined and unavailable on the client. Key behaviors: - Inlining occurs only at build time; values are frozen and do not change at runtime. - Direct access like process.env.VAR works; destructuring or dynamic keys (process.env[key]) fail because DefinePlugin performs string replacement, not object creation. - Custom DefinePlugin usage is possible via next.config.js webpack(config), typically by modifying the existing DefinePlugin instance (e.g., config.plugins.find(p => p.constructor.name === 'DefinePlugin').definitions['process.env.CUSTOM'] = JSON.stringify(value)), but it's unsupported, fragile across Next.js versions, and risks conflicts with internal defines. Next.js recommends using NEXT_PUBLIC_ prefix or next.config.js env for client exposure instead of manual DefinePlugin.

Citations:


Critical: passing process.env to Zod prevents NEXT_PUBLIC_* inlining and crashes the admin app on client load.

Next.js's DefinePlugin performs string replacement of direct property accesses like process.env.NEXT_PUBLIC_API_URL, but does not inline the process.env object itself. When adminEnvSchema.parse(process.env) passes the whole object, the client bundle receives an empty object {} at runtime instead of inlined values, causing Zod to throw NEXT_PUBLIC_API_URL: Required immediately on module import.

Since apps/admin/app/login/page.tsx is a 'use client' component that imports this module, the entire login page fails to mount in the browser—directly contradicting the PR's goal of restoring Basic-auth login. Additionally, a synchronous crash at module load removes any opportunity for graceful error handling.

Fix by reading variables directly so DefinePlugin can substitute them:

🛠️ Proposed fix
 import { z } from 'zod';

 const adminEnvSchema = z.object({
   NEXT_PUBLIC_API_URL: z.string().url(),
 });

 export type AdminEnv = z.infer<typeof adminEnvSchema>;

-/**
- * Typed env parsed from `process.env` at module load. Throws a Zod
- * validation error if any value fails its schema constraint.
- */
-export const adminEnv = adminEnvSchema.parse(process.env);
+/**
+ * Typed env parsed at module load. Each `NEXT_PUBLIC_*` is read via a direct
+ * `process.env.<NAME>` access so Next.js inlines it into the client bundle.
+ * Throws a Zod validation error if any value fails its schema constraint.
+ */
+export const adminEnv = adminEnvSchema.parse({
+  NEXT_PUBLIC_API_URL: process.env.NEXT_PUBLIC_API_URL,
+});
📝 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.

Suggested change
export const adminEnv = adminEnvSchema.parse(process.env);
import { z } from 'zod';
const adminEnvSchema = z.object({
NEXT_PUBLIC_API_URL: z.string().url(),
});
export type AdminEnv = z.infer<typeof adminEnvSchema>;
/**
* Typed env parsed at module load. Each `NEXT_PUBLIC_*` is read via a direct
* `process.env.<NAME>` access so Next.js inlines it into the client bundle.
* Throws a Zod validation error if any value fails its schema constraint.
*/
export const adminEnv = adminEnvSchema.parse({
NEXT_PUBLIC_API_URL: process.env.NEXT_PUBLIC_API_URL,
});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/admin/lib/env.ts` at line 21, The current adminEnv export passes the
entire process.env into adminEnvSchema.parse, which prevents Next.js
DefinePlugin from inlining NEXT_PUBLIC_* values and causes a runtime Zod crash
on client import; instead, change the initialization of adminEnv to pass an
object of individually read env vars (e.g. process.env.NEXT_PUBLIC_API_URL,
process.env.NEXT_PUBLIC_SOME_OTHER_VAR, etc.) into adminEnvSchema.parse so each
NEXT_PUBLIC_* access is a direct property access that will be substituted at
build time; update the code that references adminEnvSchema and the exported
adminEnv constant to construct the parsed object from explicit
process.env.<NAME> reads rather than passing process.env wholesale.

Comment on lines 9 to 14
const generatePacks = async (request: GenerationRequest): Promise<Pack[]> => {
const { data, error } = await apiClient.packs['generate-packs'].post(request);
if (error) throw new Error(`Failed to generate packs: ${error.value}`);
// safe-cast: treaty response shape matches Pack[] as validated by the API schema
return (data ?? []) as unknown as Pack[];
};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check API client type definition and similar hook patterns

# Find the API client definition to verify return type
echo "=== API client definition ==="
rg -n -A 10 "packs.*generate-packs" --type ts -g '!**/node_modules/**'

# Check similar hooks for casting patterns
echo -e "\n=== Similar hooks casting patterns ==="
fd -e ts -e tsx . apps/expo/features --exec rg -n "as unknown as" {} \;

# Find Pack type definition
echo -e "\n=== Pack type definition ==="
rg -n "^export (type|interface) Pack" --type ts -g '!**/node_modules/**'

Repository: PackRat-AI/PackRat

Length of output: 9147


🏁 Script executed:

cat -n apps/expo/lib/api/packrat.ts

Repository: PackRat-AI/PackRat

Length of output: 1699


🏁 Script executed:

# Search for treaty/API client related files
fd -e ts -e tsx . apps/expo/lib/api --type f

Repository: PackRat-AI/PackRat

Length of output: 118


🏁 Script executed:

# Check if there's an OpenAPI spec or treaty definitions
fd -e yaml -e yml -e json . packages/ -name "*treaty*" -o -name "*openapi*" -o -name "*swagger*" 2>/dev/null | head -20

Repository: PackRat-AI/PackRat

Length of output: 44


🏁 Script executed:

# Look for other uses of the generate-packs endpoint
rg "generate-packs" --type ts --type tsx -B 5 -A 5

Repository: PackRat-AI/PackRat

Length of output: 90


🏁 Script executed:

# Find the api-client package
fd -type d "api-client" packages/

Repository: PackRat-AI/PackRat

Length of output: 233


🏁 Script executed:

# Look for the createApiClient function definition
rg -A 20 "export.*createApiClient" --type ts

Repository: PackRat-AI/PackRat

Length of output: 3981


🏁 Script executed:

# Search for Treaty routes/schema definition
rg "generate-packs" packages/api --type ts -B 10 -A 10

Repository: PackRat-AI/PackRat

Length of output: 3699


🏁 Script executed:

# Check the packages/api structure to understand server routes
ls -la packages/api/src/routes/ 2>/dev/null | head -20

Repository: PackRat-AI/PackRat

Length of output: 1184


🏁 Script executed:

# Find PackService and its generatePacks method
rg -A 20 "class PackService" packages/api/src --type-list | head -30

Repository: PackRat-AI/PackRat

Length of output: 452


🏁 Script executed:

# Search for generatePacks implementation
rg -B 5 -A 20 "generatePacks\(" packages/api/src --type ts -g '!**/test/**'

Repository: PackRat-AI/PackRat

Length of output: 4964


🏁 Script executed:

# Check if other route handlers define response schemas
rg -B 5 -A 10 "response: z\." packages/api/src/routes --type ts | head -50

Repository: PackRat-AI/PackRat

Length of output: 44


🏁 Script executed:

# Look at the full packs route configuration to see if there's a response schema
sed -n '1,200p' packages/api/src/routes/packs/index.ts

Repository: PackRat-AI/PackRat

Length of output: 5916


🏁 Script executed:

# Get more of the PackService.generatePacks implementation to see its return type
rg -B 2 -A 50 "async generatePacks\(count: number\)" packages/api/src/services/packService.ts

Repository: PackRat-AI/PackRat

Length of output: 1615


🏁 Script executed:

# Check what types are returned by the database operations
rg -A 30 "createdPacks = await this\.db\.transaction" packages/api/src/services/packService.ts

Repository: PackRat-AI/PackRat

Length of output: 1010


🏁 Script executed:

# Compare with an endpoint that DOES define a response schema
rg -B 5 -A 15 "response:" packages/api/src/routes --type ts | head -40

Repository: PackRat-AI/PackRat

Length of output: 1590


🏁 Script executed:

# Check if other endpoints with doublecast also lack response schemas
rg -B 10 "as unknown as" apps/expo/features --type ts | grep -A 10 -B 10 "response:" | head -50

Repository: PackRat-AI/PackRat

Length of output: 44


🏁 Script executed:

# Check if any Elysia routes define response schemas
rg "response:" packages/api/src/routes -A 2 -B 2 --type ts | head -30

Repository: PackRat-AI/PackRat

Length of output: 328


🏁 Script executed:

# Check if the PackSchema is exported and available
rg -A 5 "export.*PackSchema" packages/api/src --type ts

Repository: PackRat-AI/PackRat

Length of output: 2375


🏁 Script executed:

# Verify what type the database insert returns
rg -B 5 -A 10 "insert\(packs\)\.values" packages/api/src --type ts

Repository: PackRat-AI/PackRat

Length of output: 1173


🏁 Script executed:

# Check other route handlers to confirm if response schemas are used elsewhere
ls packages/api/src/routes/*.ts | xargs -I {} sh -c 'echo "=== {} ===" && head -100 {}'

Repository: PackRat-AI/PackRat

Length of output: 18477


🏁 Script executed:

# Check the Elysia/Treaty documentation in comments
rg -B 5 -A 10 "response:" packages/api/src --type ts -A 5

Repository: PackRat-AI/PackRat

Length of output: 2454


🏁 Script executed:

# Check if there's a pattern with generated types from Treaty
rg "Treaty" packages/api-client --type ts -B 2 -A 2

Repository: PackRat-AI/PackRat

Length of output: 2265


🏁 Script executed:

# See the full context of how routes are configured in Elysia
cat -n packages/api/src/routes/packs/index.ts | sed -n '1,50p'

Repository: PackRat-AI/PackRat

Length of output: 1954


Add response schema to the API route to properly type the response.

The double cast as unknown as Pack[] works around a missing API contract definition. The /packs/generate-packs endpoint does not define a response schema in its Elysia route config (only body and detail are specified). Without a response schema, Treaty cannot infer the response type and defaults to unknown.

Instead of casting on the client, add a response schema to the server route:

Suggested fix at packages/api/src/routes/packs/index.ts
.post(
  '/generate-packs',
  async ({ body, user }) => {
    const packService = new PackService(user.userId);
    return packService.generatePacks(body.count);
  },
  {
    body: z.object({ count: z.number().int().positive().default(1) }),
    response: z.array(selectPackSchema),  // Add response schema
    isAdmin: true,
    detail: { ... },
  },
)

Once the server-side response schema is in place, Treaty will properly type the response and the client-side cast can be removed.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/expo/features/ai-packs/hooks/useGeneratedPacks.ts` around lines 9 - 14,
The client double-casts response to Pack[] in generatePacks because the server
route for '/generate-packs' lacks a response schema; update the server route
handler (the .post('/generate-packs', ...) in the packs route) to include
response: z.array(selectPackSchema) (or the appropriate zod Pack schema) in its
route options, then remove the unsafe cast in the client function generatePacks
so Treaty can infer the correct Pack[] type from the server schema.

// the cast is verified by the switch guard above each arm.
switch (toolInvocation.type) {
case 'tool-webSearchTool':
// safe-cast: case guard narrows type to discriminant; local tool types extend ToolUIPart with that exact `type` field
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Line exceeds 100-character width limit.

Line 25 is approximately 126 characters, violating the 100-character line width enforced by Biome 2.0 for this file path. As per coding guidelines, enforce 2 spaces indentation, 100 character line width, and single quotes via Biome 2.0 for **/*.{js,jsx,ts,tsx,json}.

✂️ Proposed fix to split the comment
-      // safe-cast: case guard narrows type to discriminant; local tool types extend ToolUIPart with that exact `type` field
+      // safe-cast: case guard narrows type to discriminant; local tool types extend
+      // ToolUIPart with that exact `type` field
       return <WebSearchGenerativeUI toolInvocation={toolInvocation as WebSearchTool} />;
📝 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.

Suggested change
// safe-cast: case guard narrows type to discriminant; local tool types extend ToolUIPart with that exact `type` field
// safe-cast: case guard narrows type to discriminant; local tool types extend
// ToolUIPart with that exact `type` field
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/expo/features/ai/components/ToolInvocationRenderer.tsx` at line 25, The
inline comment inside the ToolInvocationRenderer component ("// safe-cast: case
guard narrows type to discriminant; local tool types extend ToolUIPart with that
exact `type` field") exceeds the 100-character width; split it into two or more
shorter comment lines under 100 characters (preserve the same wording and
meaning), keep the file's 2-space indentation, and ensure any string literals in
nearby code use single quotes per Biome rules.

import { Elysia, status } from 'elysia';
import { z } from 'zod';

const OsmSportSchema = z.enum(['hiking', 'cycling', 'skiing', 'running', 'horse_riding']);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Inline OsmSportSchema will drift from OsmSport exported by @packrat/overpass.

OsmSport is the source-of-truth union in @packrat/overpass, and OSM_SPORT_MAP in builder.ts is keyed by it. Hard-coding the same literals here means a new sport added to the package won't surface in this route until someone remembers to update both lists, and a typo here silently rejects a valid sport at the edge.

Either derive the schema from the package type, or export a zod schema from @packrat/overpass and import it.

♻️ Suggested approach (export schema from package)

In packages/overpass/src/types.ts (or a new schemas export):

export const OsmSportSchema = z.enum(['hiking', 'cycling', 'skiing', 'running', 'horse_riding']);
export type OsmSport = z.infer<typeof OsmSportSchema>;

Then in this file:

-import { queryOverpass, TrailQueryBuilder, toTrailDetail, toTrailSummary } from '@packrat/overpass';
+import {
+  OsmSportSchema,
+  queryOverpass,
+  TrailQueryBuilder,
+  toTrailDetail,
+  toTrailSummary,
+} from '@packrat/overpass';
@@
-const OsmSportSchema = z.enum(['hiking', 'cycling', 'skiing', 'running', 'horse_riding']);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/api/src/routes/trails/index.ts` at line 6, Replace the hard-coded
local OsmSportSchema with the canonical schema exported from `@packrat/overpass`:
add/export a Zod schema (e.g., OsmSportSchema) alongside the OsmSport type in
the overpass package (or export an already-derived schema), then import and use
that OsmSportSchema here instead of the inline z.enum; ensure the route uses the
same symbol as OSM_SPORT_MAP in builder.ts (OsmSport/OsmSportSchema) so new
sports added to `@packrat/overpass` automatically flow through.

algorithms: ['HS256'],
});
return payload as JWTPayload;
return payload as JWTPayload; // safe-cast: jose jwtVerify returns JWTPayload — our JWTPayload type extends the jose type with userId/role fields
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify call sites that assume verifyJWT() always returns payload with numeric userId
rg -n -C3 --type=ts '\bverifyJWT\s*\('
rg -n -C3 --type=ts '\buserId\b'

Repository: PackRat-AI/PackRat

Length of output: 50375


🏁 Script executed:

cat -n packages/api/src/utils/auth.ts | head -80

Repository: PackRat-AI/PackRat

Length of output: 2916


🏁 Script executed:

cat -n packages/api/src/middleware/auth.ts | head -80

Repository: PackRat-AI/PackRat

Length of output: 3478


🏁 Script executed:

rg -A5 "export.*isNumber" packages/

Repository: PackRat-AI/PackRat

Length of output: 44


🏁 Script executed:

rg "isNumber" packages/api/src/utils/auth.ts -B2 -A2

Repository: PackRat-AI/PackRat

Length of output: 468


🏁 Script executed:

cat -n packages/api/src/utils/auth.ts | sed -n '1,13p'

Repository: PackRat-AI/PackRat

Length of output: 513


🏁 Script executed:

rg "type.*payload|interface.*payload" node_modules/jose --type=ts -A3 2>/dev/null | head -30

Repository: PackRat-AI/PackRat

Length of output: 44


🏁 Script executed:

rg "generateJWT" packages/api/src -B2 -A5 -m3

Repository: PackRat-AI/PackRat

Length of output: 3554


🏁 Script executed:

cat -n packages/api/src/utils/__tests__/auth.test.ts | sed -n '75,95p'

Repository: PackRat-AI/PackRat

Length of output: 919


Add runtime validation for JWT payload claims before returning.

Line 67 casts payload to JWTPayload without verifying that userId is numeric or that role (if present) is valid. Since jwtVerify returns unknown type, an attacker could craft a token with missing or malformed claims, breaking downstream code that assumes these fields exist and are correctly typed.

Validate claims with a type guard before returning:

Suggested fix
+function isValidAuthPayload(payload: unknown): payload is JWTPayload {
+  if (!payload || typeof payload !== 'object') return false;
+  const p = payload as Record<string, unknown>;
+  if (!isNumber(p.userId)) return false;
+  if (p.role !== undefined && p.role !== 'USER' && p.role !== 'ADMIN') return false;
+  if (p.exp !== undefined && !isNumber(p.exp)) return false;
+  if (p.iat !== undefined && !isNumber(p.iat)) return false;
+  return true;
+}
+
 export async function verifyJWT({ token }: { token: string }): Promise<JWTPayload | null> {
   try {
     const { JWT_SECRET } = getEnv();
     const { payload } = await jwtVerify(token, secretKey(JWT_SECRET), {
       algorithms: ['HS256'],
     });
-    return payload as JWTPayload; // safe-cast: jose jwtVerify returns JWTPayload — our JWTPayload type extends the jose type with userId/role fields
+    if (!isValidAuthPayload(payload)) return null;
+    return payload;
   } catch {
     return null;
   }
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/api/src/utils/auth.ts` at line 67, The return currently unsafely
casts payload to JWTPayload; update the function that calls jwtVerify (e.g.,
verifyJwt / the function in auth.ts that returns JWTPayload) to perform a
runtime type guard: assert payload is an object, that payload.userId exists and
is a number (or a numeric string converted/validated), and if payload.role
exists it matches the expected role enum/strings; if validation fails, throw a
descriptive Auth/Validation error instead of casting. Replace the direct cast
"return payload as JWTPayload" with this guard + normalized fields (convert
numeric strings to numbers if intended) so downstream code can safely rely on
userId/role types.

Comment on lines +25 to +29
name(q: string): this {
const escaped = q.replace(RE_BACKSLASH, '\\\\').replace(RE_DOUBLE_QUOTE, '\\"');
this._filters.push(`["name"~"${escaped}",i]`);
return this;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

name() doesn't escape regex metacharacters — it leaves user input as a regex.

Overpass treats the right-hand side of ~"..." as a POSIX regex. Escaping only \ and " means inputs containing ., *, +, ?, (, ), [, ], {, }, ^, $, or | produce wrong matches or — for unbalanced groups/brackets — a server-side runtime error. Real names trigger this routinely:

  • name('Park (East)')(East) becomes a capture group
  • name('a+b')+ quantifies a
  • name('Mt. Whitney'). matches any character

Either escape regex metacharacters before emitting, or anchor an explicit literal-substring search.

🛡️ Proposed fix
 const RE_BACKSLASH = /\\/g;
 const RE_DOUBLE_QUOTE = /"/g;
+const RE_REGEX_META = /[.*+?^${}()|[\]]/g;
@@
   name(q: string): this {
-    const escaped = q.replace(RE_BACKSLASH, '\\\\').replace(RE_DOUBLE_QUOTE, '\\"');
+    const escaped = q
+      .replace(RE_BACKSLASH, '\\\\')
+      .replace(RE_REGEX_META, '\\$&')
+      .replace(RE_DOUBLE_QUOTE, '\\"');
     this._filters.push(`["name"~"${escaped}",i]`);
     return this;
   }

Don't forget to add a corresponding test in builder.test.ts (e.g., name('Mt. Whitney (East)')).

📝 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.

Suggested change
name(q: string): this {
const escaped = q.replace(RE_BACKSLASH, '\\\\').replace(RE_DOUBLE_QUOTE, '\\"');
this._filters.push(`["name"~"${escaped}",i]`);
return this;
}
name(q: string): this {
const escaped = q
.replace(RE_BACKSLASH, '\\\\')
.replace(RE_REGEX_META, '\\$&')
.replace(RE_DOUBLE_QUOTE, '\\"');
this._filters.push(`["name"~"${escaped}",i]`);
return this;
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/overpass/src/builder.ts` around lines 25 - 29, The name(q: string)
method currently only escapes backslashes and double quotes, leaving user input
as a regex which allows metacharacters (.,*,+,?,(),[],{},^,$,|) to change
semantics or cause runtime errors; update name to escape all regex
metacharacters before pushing to this._filters (or alternatively build the
filter as a literal substring search) so inputs like "Mt. Whitney (East)" are
treated literally; modify the escaping logic that uses RE_BACKSLASH and
RE_DOUBLE_QUOTE (or replace it) to include a pass that escapes . * + ? ( ) [ ] {
} ^ $ | and ensure tests in builder.test.ts (e.g., name('Mt. Whitney (East)'))
are added/updated to cover these cases.

Comment on lines +43 to +46
id(osmId: number | string | bigint): this {
this._byId = String(osmId);
return this;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Validate the string form of id() for defense in depth.

String(osmId) is interpolated unescaped into relation(${this._byId}). Today the only caller (/trails/:osmId) coerces via Number() first, but the public type signature accepts string, so a future caller that forwards user input directly would enable Overpass QL injection (e.g., "12);out;node(1"). Reject anything that isn't a positive integer literal at the builder boundary.

🛡️ Proposed fix
+const RE_POSITIVE_INT = /^[1-9]\d*$/;
+
@@
   id(osmId: number | string | bigint): this {
-    this._byId = String(osmId);
-    return this;
+    const s = String(osmId);
+    if (!RE_POSITIVE_INT.test(s)) {
+      throw new Error(`Invalid osmId: ${s}`);
+    }
+    this._byId = s;
+    return this;
   }
📝 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.

Suggested change
id(osmId: number | string | bigint): this {
this._byId = String(osmId);
return this;
}
const RE_POSITIVE_INT = /^[1-9]\d*$/;
id(osmId: number | string | bigint): this {
const s = String(osmId);
if (!RE_POSITIVE_INT.test(s)) {
throw new Error(`Invalid osmId: ${s}`);
}
this._byId = s;
return this;
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/overpass/src/builder.ts` around lines 43 - 46, The id(osmId: number
| string | bigint) setter currently does String(osmId) and can allow unsafe
inputs; change it to validate the value is a positive integer literal before
assigning this._byId: accept numbers and bigints by coercing to string, and for
strings ensure they match /^\d+$/ (no signs, spaces or other chars); if
validation fails throw a descriptive Error and do not set this._byId (this
protects the later relation(${this._byId}) interpolation). Ensure the validation
logic is implemented inside the id(...) method and used consistently before
assigning this._byId.

Comment on lines +17 to +24
const response = await fetch(endpoint, {
method: 'POST',
headers: {
'Content-Type': 'application/x-www-form-urlencoded',
'User-Agent': USER_AGENT,
},
body: `data=${encodeURIComponent(ql)}`,
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add an AbortSignal timeout to the fetch call.

The Overpass QL preamble sets a server-side timeout, but the HTTP request itself has no client-side timeout. A stalled connection (slow DNS, dead TCP, partial response) will hold the request open until the runtime forcibly kills it. On Cloudflare Workers this consumes the request budget; on long-lived runtimes it can leak handles.

🛡️ Proposed fix
-export interface OverpassClientConfig {
-  endpoint?: string;
-}
+export interface OverpassClientConfig {
+  endpoint?: string;
+  /** Hard client-side request timeout in milliseconds (default 30s). */
+  timeoutMs?: number;
+}
@@
-  const endpoint = config?.endpoint ?? DEFAULT_ENDPOINT;
-
-  const response = await fetch(endpoint, {
+  const endpoint = config?.endpoint ?? DEFAULT_ENDPOINT;
+  const timeoutMs = config?.timeoutMs ?? 30_000;
+
+  const response = await fetch(endpoint, {
     method: 'POST',
     headers: {
       'Content-Type': 'application/x-www-form-urlencoded',
       'User-Agent': USER_AGENT,
     },
     body: `data=${encodeURIComponent(ql)}`,
+    signal: AbortSignal.timeout(timeoutMs),
   });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/overpass/src/client.ts` around lines 17 - 24, The fetch call that
posts Overpass QL (the const response = await fetch(endpoint, { ... body:
`data=${encodeURIComponent(ql)}` }) ) needs a client-side timeout: create an
AbortController, pass controller.signal in the fetch options, start a setTimeout
to call controller.abort() after a reasonable timeout (e.g. 15–30s), and clear
the timeout once fetch resolves or throws; ensure the signal is used in the same
fetch invocation and handle the aborted error path appropriately (reference:
response, endpoint, ql, USER_AGENT).

Comment on lines +44 to +49
// Files explicitly allowed to use raw regex.
// alltrails.ts: builds regex from a dynamic `property` argument — can't be a static constant.
const EXCLUDED_FILES = new Set([
'packages/analytics/src/core/enrichment.ts',
'packages/api/src/routes/alltrails.ts',
]);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Inspect raw regex usages in alltrails.ts to confirm dynamic input is escaped.
fd -t f 'alltrails.ts' packages/api/src/routes -x cat {}
echo "---"
rg -nP -C3 '(new\s+RegExp|/[^/\n]*\$\{|escape(RegExp|Regex))' packages/api/src/routes/alltrails.ts || true

Repository: PackRat-AI/PackRat

Length of output: 3844


Dynamic property argument in extractOgTag must be escaped before regex interpolation.

The function at lines 8–15 embeds the property parameter directly into regex patterns without escaping:

const match =
  html.match(
    new RegExp(`<meta[^>]+property=["']${property}["'][^>]+content=["']([^"']+)["']`, 'i'),
  ) ??
  html.match(
    new RegExp(`<meta[^>]+content=["']([^"']+)["'][^>]+property=["']${property}["']`, 'i'),
  );

This creates a regex injection vulnerability. Use escapeRegExp (or equivalent) to safely escape metacharacters before embedding property in the pattern.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/lint/no-raw-regex.ts` around lines 44 - 49, The extractOgTag function
interpolates the dynamic property parameter directly into RegExp, which allows
regex injection; fix by escaping property before using it in new RegExp (e.g.,
add or import an escapeRegExp utility and call it on property), then use the
escaped value in both RegExp constructors inside extractOgTag so metacharacters
are neutralized and patterns remain safe.

@andrew-bierman
Copy link
Copy Markdown
Collaborator Author

Closing — the original strict CF-only guard is correct. Root cause is getCFAccessIdentity() returning null after valid CF session; fixing that instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api ci/cd dependencies Pull requests that update a dependency file mobile web

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants