feat: switch deploy from Cloudflare Pages to local tunnel#122
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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 configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (25)
📝 WalkthroughWalkthroughThe deployment architecture fundamentally shifts from GitHub Actions scheduled jobs and Cloudflare Pages static export to a self-hosted Docker Compose stack. Terraform now provisions Cloudflare Zero Trust tunnel and Access policies. The crawler moves into a containerized service scheduled via supercronic. The web application gains a cache revalidation API endpoint and conditionally uses static export based on environment variables. Changes
Sequence DiagramsequenceDiagram
actor User
participant Access as Cloudflare<br/>Zero Trust Access
participant Tunnel as Cloudflare<br/>Tunnel (cloudflared)
participant Web as Web Service<br/>(Next.js 8765)
participant Crawler as Crawler Service<br/>(supercronic)
participant DB as SQLite DB<br/>(shared volume)
Note over Crawler: Scheduled Time (supercronic)
Crawler->>Crawler: run-crawl.sh starts
Crawler->>DB: Query/Update moneyforward data
DB-->>Crawler: Data updated
Crawler->>Web: POST /api/refresh<br/>Bearer: REFRESH_TOKEN
Web->>Web: Authenticate token
Web->>Web: revalidatePath("/", "layout")
Web-->>Crawler: { revalidated: true }
Crawler->>Crawler: Log completion
User->>Access: Request hostname
Access->>Access: Check email allowlist<br/>(Google IdP)
Access->>Tunnel: Forward authenticated request
Tunnel->>Web: Route to localhost:8765
Web-->>Tunnel: Render fresh content
Tunnel-->>Access: Response
Access-->>User: Serve cached/fresh page
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes The changes span multiple heterogeneous domains—Terraform infrastructure with conditional logic, Docker containerization with multi-stage builds and entrypoints, new API endpoint with authentication, environment configuration, and documentation—requiring separate reasoning for each cohort. While no single file is trivial, the complexity is distributed across interconnected systems rather than concentrated in dense logic blocks. Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 6 minutes and 21 seconds.Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
terraform/versions.tf (1)
2-2: Use pessimistic constraint operator for safer version pinning.On Line 2,
>= 1.6permits unintended major version upgrades. HashiCorp's official recommendation is to use the pessimistic constraint operator~>which pins the minor version while allowing patch-level updates. This is the standard pattern for production IaC repositories.Proposed refinement
- required_version = ">= 1.6" + required_version = "~> 1.6"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@terraform/versions.tf` at line 2, Change the Terraform required_version constraint from the open-ended ">= 1.6" to the pessimistic operator form to pin the minor version (replace the value used for required_version so it reads using "~> 1.6"); update the required_version assignment in the same block where required_version is defined to use "~> 1.6" instead of ">= 1.6".terraform/variables.tf (1)
37-41: Consider adding validation forsession_durationformat.Cloudflare Access accepts specific duration formats (e.g.,
30m,6h,24h). Adding a regex validation could catch invalid formats early.Optional validation for session_duration
variable "session_duration" { description = "Cloudflare Access session duration" type = string default = "24h" + validation { + condition = can(regex("^\\d+(m|h)$", var.session_duration)) + error_message = "session_duration must be in format like '30m' or '24h'." + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@terraform/variables.tf` around lines 37 - 41, Add a validation block to the Terraform variable "session_duration" to ensure values match Cloudflare Access duration formats (e.g., minutes or hours like 30m, 6h, 24h); update the variable "session_duration" declaration by adding a validation with a regex that enforces the allowed pattern (for example digits followed by m or h) and provide a clear error_message explaining the accepted formats so invalid inputs are caught during plan/apply.docs/setup.md (1)
121-129: Consider providing a sample launchd plist template.While noting that "plist の内容は環境依存のため本リポジトリには含めない" is reasonable, providing a template or example in the docs could help users get started more quickly. This is optional.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/setup.md` around lines 121 - 129, Add a minimal, copy-pasteable launchd plist template to the "launchd 常駐 (Mac)" section in docs/setup.md for the three referenced plists (me.hiroppy.mf-dashboard.serve.plist, me.hiroppy.mf-dashboard.tunnel.plist, me.hiroppy.mf-dashboard.crawler.plist), using placeholders for absolute paths, the user, and ProgramArguments, and include an example StartCalendarInterval entry for the crawler’s JST 6:50 and 15:20 schedule; keep the template annotated with which fields to replace and a short note that the files are environment-specific and should be adjusted before use.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/serve-web.sh`:
- Line 16: The serve command is passing a bare port to --listen which expects a
URI; update the exec invocation that runs pnpm dlx serve (the line using exec
pnpm dlx serve "${OUT_DIR}" --listen "${PORT}" --single) to specify the port
correctly by either using the -p "${PORT}" flag or formatting the listen value
as a full host:port (e.g., 0.0.0.0:${PORT}); keep the --single flag unchanged.
In `@scripts/start-tunnel.sh`:
- Around line 18-19: The script uses TUNNEL_TOKEN_REF and TUNNEL_TOKEN with op
read ("op://Private/Cloudflare Tunnel mf-dashboard/credential") but the
terraform/docs and 1Password CLI field syntax are inconsistent; fix by aligning
storage and retrieval: either change the terraform/docs to store the token under
the top-level field name "credential" (so op read "op://.../credential" is
correct) or update TUNNEL_TOKEN_REF to the actual field path used by 1Password
(e.g., "op://Private/Cloudflare Tunnel mf-dashboard/credential/password" or
"op://.../password" depending on whether you stored it as a section) and update
the op read call accordingly; ensure TUNNEL_TOKEN_REF and the op read invocation
reference the exact same 1Password field name so TUNNEL_TOKEN receives the
stored token.
In `@terraform/.env.template`:
- Line 1: The CLOUDFLARE_API_TOKEN value in the env template contains spaces and
must be quoted for the 1Password CLI to parse it correctly; update the
CLOUDFLARE_API_TOKEN assignment to wrap the credential reference in quotes
(e.g., CLOUDFLARE_API_TOKEN="op://Private/Cloudflare API Token
mf-dashboard/credential") so the entire string, including spaces, is preserved
when using op run --env-file.
In `@terraform/main.tf`:
- Around line 50-71: The inline policy in resource
cloudflare_zero_trust_access_application.this uses an incorrect nested include
object ({ email = { email = email } }); replace that with a single include
object that uses the documented emails attribute and passes var.allowed_emails
(i.e., set policies -> include to a single map with emails = var.allowed_emails)
so the policy uses the provider-supported structure for allowed emails.
---
Nitpick comments:
In `@docs/setup.md`:
- Around line 121-129: Add a minimal, copy-pasteable launchd plist template to
the "launchd 常駐 (Mac)" section in docs/setup.md for the three referenced plists
(me.hiroppy.mf-dashboard.serve.plist, me.hiroppy.mf-dashboard.tunnel.plist,
me.hiroppy.mf-dashboard.crawler.plist), using placeholders for absolute paths,
the user, and ProgramArguments, and include an example StartCalendarInterval
entry for the crawler’s JST 6:50 and 15:20 schedule; keep the template annotated
with which fields to replace and a short note that the files are
environment-specific and should be adjusted before use.
In `@terraform/variables.tf`:
- Around line 37-41: Add a validation block to the Terraform variable
"session_duration" to ensure values match Cloudflare Access duration formats
(e.g., minutes or hours like 30m, 6h, 24h); update the variable
"session_duration" declaration by adding a validation with a regex that enforces
the allowed pattern (for example digits followed by m or h) and provide a clear
error_message explaining the accepted formats so invalid inputs are caught
during plan/apply.
In `@terraform/versions.tf`:
- Line 2: Change the Terraform required_version constraint from the open-ended
">= 1.6" to the pessimistic operator form to pin the minor version (replace the
value used for required_version so it reads using "~> 1.6"); update the
required_version assignment in the same block where required_version is defined
to use "~> 1.6" instead of ">= 1.6".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5372bd95-d041-4d1c-af1b-ab51d5769d89
📒 Files selected for processing (17)
.github/workflows/daily-update.yml.gitignoreREADME.mddocs/setup.mdpackage.jsonscripts/serve-web.shscripts/start-tunnel.shterraform/.env.templateterraform/.gitignoreterraform/.terraform.lock.hclterraform/README.mdterraform/main.tfterraform/outputs.tfterraform/terraform.tfvars.exampleterraform/variables.tfterraform/versions.tfwrangler.toml
💤 Files with no reviewable changes (1)
- wrangler.toml
| exit 1 | ||
| fi | ||
|
|
||
| exec pnpm dlx serve "${OUT_DIR}" --listen "${PORT}" --single |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check serve package help for correct port flag syntax
pnpm dlx serve --help 2>&1 | head -40Repository: hiroppy/mf-dashboard
Length of output: 1490
Use -p flag or provide full URI to --listen for port specification.
The serve package expects --listen to receive a URI endpoint (e.g., 0.0.0.0:8765), not a bare port number. Use -p "${PORT}" instead, or format as --listen 0.0.0.0:${PORT}.
Suggested fix:
exec pnpm dlx serve "${OUT_DIR}" -p "${PORT}" --single
The --single flag usage is correct.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/serve-web.sh` at line 16, The serve command is passing a bare port to
--listen which expects a URI; update the exec invocation that runs pnpm dlx
serve (the line using exec pnpm dlx serve "${OUT_DIR}" --listen "${PORT}"
--single) to specify the port correctly by either using the -p "${PORT}" flag or
formatting the listen value as a full host:port (e.g., 0.0.0.0:${PORT}); keep
the --single flag unchanged.
| resource "cloudflare_zero_trust_access_application" "this" { | ||
| account_id = var.account_id | ||
| name = var.tunnel_name | ||
| domain = var.hostname | ||
| type = "self_hosted" | ||
| session_duration = var.session_duration | ||
| allowed_idps = local.google_idp_id != null ? [local.google_idp_id] : [] | ||
| auto_redirect_to_identity = local.google_idp_id != null | ||
|
|
||
| policies = [ | ||
| { | ||
| name = "Allow specified emails" | ||
| decision = "allow" | ||
| precedence = 1 | ||
| include = [ | ||
| for email in var.allowed_emails : { | ||
| email = { email = email } | ||
| } | ||
| ] | ||
| } | ||
| ] | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Cloudflare Terraform provider v5 cloudflare_zero_trust_access_application policies syntax
💡 Result:
In Cloudflare Terraform provider v5 (e.g., v5.15.0+), the cloudflare_zero_trust_access_application resource uses a policies attribute that is a list of policy objects applied to the application in ascending order of precedence. Each policy item can reference an existing policy by id (UUID) and specify precedence (unique number per app), or define a new app-exclusive policy inline with full details including name, decision, include, exclude, require, connection_rules, etc. Example from official docs (referencing existing policy): policies = [{ id = "f174e90a-fafe-4643-bbbc-4a0ed4fc8415" precedence = 0 }] Nested schema for policies (each item): - id (String, optional): UUID of existing policy - name (String): Name of policy (required for new) - precedence (Number): Execution order (unique per app) - decision (String): "allow", "deny", "non_identity", "bypass" (allow only for infra apps) - include (Set of Attributes): OR rules (user matches one); sub-attributes: emails, email_domains, ip, geo, group, gsuite, azure_ad, saml, okta, github_organization, service_token, everyone, etc. - exclude (Set of Attributes): NOT rules (user matches none) - require (Set of Attributes): AND rules (user matches all) - connection_rules (Attributes): Protocol-specific rules (ssh, rdp, etc.) - Additional: approval_required, approval_groups, session_duration, isolation_required, purpose_justification_required, mfa_config (newer versions) For full inline policy example, create separate cloudflare_zero_trust_access_policy first (reusable across apps if no app_id), then reference: resource "cloudflare_zero_trust_access_policy" "example" { account_id = "account_id" name = "Allow devs" decision = "allow" include = [{ group = { id = "group_uuid" } }] require = [{ group = { id = "group_uuid" } }] } resource "cloudflare_zero_trust_access_application" "example" { # ... other config policies = [{ id = cloudflare_zero_trust_access_policy.example.id precedence = 1 }] } Policies are evaluated top-to-bottom by precedence. Official docs: https://registry.terraform.io/providers/cloudflare/cloudflare/5.15.0/docs/resources/zero_trust_access_application and https://registry.terraform.io/providers/cloudflare/cloudflare/5.15.0/docs/resources/zero_trust_access_policy
Citations:
- 1: https://registry.terraform.io/providers/cloudflare/cloudflare/5.15.0/docs/resources/zero_trust_access_application
- 2: https://registry.terraform.io/providers/cloudflare/cloudflare/5.15.0/docs/data-sources/zero_trust_access_policy
- 3: https://registry.terraform.io/providers/cloudflare/cloudflare/5.0.0-rc1/docs/resources/zero_trust_access_policy
- 4: https://linkedin.com/company/%e9%9b%bb%e7%a2%bc%e7%a7%91%e6%8a%80%e6%9c%89%e9%99%90%e5%85%ac%e5%8f%b8
- 5: https://registry.terraform.io/providers/cloudflare/cloudflare/5.18.0/docs/resources/zero_trust_access_policy
Fix inline policy include structure to use documented emails attribute.
The include block is using an incorrect nested structure { email = { email = email } }. Per Cloudflare Terraform Provider v5 documentation, the include attribute accepts sub-attributes including emails, email_domains, group, geo, etc. The code should consolidate all emails into a single include object:
Corrected policy structure
policies = [
{
name = "Allow specified emails"
decision = "allow"
precedence = 1
include = [
{
emails = var.allowed_emails
}
]
}
]
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@terraform/main.tf` around lines 50 - 71, The inline policy in resource
cloudflare_zero_trust_access_application.this uses an incorrect nested include
object ({ email = { email = email } }); replace that with a single include
object that uses the documented emails attribute and passes var.allowed_emails
(i.e., set policies -> include to a single map with emails = var.allowed_emails)
so the policy uses the provider-supported structure for allowed emails.
9810847 to
f1ad730
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
10-11:⚠️ Potential issue | 🔴 CriticalCritical: Outdated documentation contradicts new architecture.
These lines still describe the old GitHub Actions workflow with crontab, but the PR has migrated to Docker Compose with supercronic (as shown in lines 45-72). This section must be updated to reflect that scheduling now happens in the
crawlercontainer viasupercronic, not GitHub Actions.📝 Suggested update to align with new architecture
-GitHubのworkflowでcrontabを使い定期的に実行し、登録金融機関の「一括更新」ボタンを押し監視を行う。デフォルトの設定は、毎日6:50(JST)と15:20(JST)。GitHubのcrontabは指定時間ちょうどに実行されないので、-10分に設定。 +Docker Compose で常駐する crawler コンテナ内の supercronic を使い定期的に実行し、登録金融機関の「一括更新」ボタンを押し監視を行う。デフォルトの設定は、毎日6:50(JST)と15:20(JST)。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 10 - 11, Update the README paragraph that describes using GitHub Actions crontab: remove or replace the description about scheduling via GitHub workflow and crontab and clearly state that scheduling is now performed inside the crawler container using supercronic under Docker Compose; mention the configured schedules (daily 06:50 JST and 15:20 JST with the -10 minute adjustment) and point readers to the Docker Compose setup and the supercronic cron file (referencing "crawler" container and "supercronic") so it's clear where to change timing going forward.
🧹 Nitpick comments (6)
docker/crawler/Dockerfile (2)
24-29: Redundant COPY commands afterCOPY . .Lines 26-28 copy files that are already included in
COPY . .on line 24. You only need thechmodto make them executable.♻️ Proposed simplification
COPY . . -COPY docker/crawler/crontab /app/docker/crawler/crontab -COPY docker/crawler/entrypoint.sh /app/docker/crawler/entrypoint.sh -COPY docker/crawler/run-crawl.sh /app/docker/crawler/run-crawl.sh RUN chmod +x /app/docker/crawler/entrypoint.sh /app/docker/crawler/run-crawl.sh🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker/crawler/Dockerfile` around lines 24 - 29, The Dockerfile currently runs COPY . . and then redundantly re-COPYs docker/crawler/crontab, docker/crawler/entrypoint.sh, and docker/crawler/run-crawl.sh; remove those extra COPY lines and keep only the RUN chmod +x /app/docker/crawler/entrypoint.sh /app/docker/crawler/run-crawl.sh to set executables (ensure the chmod paths match where COPY . . places files, e.g., /app/docker/crawler/* or adjust the chmod target if the context places them elsewhere).
1-31: Consider adding a non-root user (lower priority).Trivy flags running as root (DS-0002). While the Playwright base image defaults to root and this runs on a controlled local environment, adding a non-root user improves security posture.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker/crawler/Dockerfile` around lines 1 - 31, Add a non-root user and switch to it after files and binaries are installed: create a user/group (e.g., appuser) and a writable app directory, chown /app and any installed binaries like /usr/local/bin/supercronic to that user after the RUN steps that install packages and copy files, then add a USER appuser (and set HOME if needed) before the ENTRYPOINT so ENTRYPOINT ["/usr/bin/tini", "--", "/app/docker/crawler/entrypoint.sh"] runs as the non-root user; ensure the existing chmod lines for /app/docker/crawler/entrypoint.sh and run-crawl.sh still apply or are performed as root before changing to USER.docker/web/Dockerfile (1)
1-27: Consider adding a non-root user for better security posture.While this container runs on a local PC with controlled access, running as a non-root user is a good security practice. This is flagged by static analysis (Trivy DS-0002).
♻️ Proposed fix to add non-root user
FROM node:22-bookworm-slim AS base WORKDIR /app ENV PNPM_HOME=/pnpm \ PATH=/pnpm:$PATH \ NODE_ENV=production RUN corepack enable && corepack prepare pnpm@10.33.0 --activate \ && apt-get update && apt-get install -y --no-install-recommends curl ca-certificates \ - && rm -rf /var/lib/apt/lists/* + && rm -rf /var/lib/apt/lists/* \ + && useradd -r -u 1001 -g root nextjs # 依存解決のキャッシュを効かせるため、まず lockfile と各 workspace の package.json だけコピー COPY pnpm-workspace.yaml pnpm-lock.yaml package.json ./ COPY apps/web/package.json ./apps/web/ COPY packages/db/package.json ./packages/db/ COPY packages/meta/package.json ./packages/meta/ COPY packages/analytics/package.json ./packages/analytics/ RUN pnpm install --frozen-lockfile --filter "@mf-dashboard/web..." # 残りのソースをコピー (.dockerignore で node_modules や .next を除外) COPY . . # data/demo.db を placeholder にして .next を生成しておく。 # 実 DB の更新は revalidatePath で反映するためここでのビルドは bootstrap 用途。 RUN DB_PATH=/app/data/demo.db pnpm --filter `@mf-dashboard/web` build +RUN chown -R nextjs:root /app/.next EXPOSE 8765 +USER nextjs CMD ["pnpm", "--filter", "@mf-dashboard/web", "start", "--port", "8765"]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker/web/Dockerfile` around lines 1 - 27, Add a non-root user and switch to it after any root-only setup: create a dedicated user/group (e.g., appuser), chown the /app workspace to that user, and add USER appuser before the final EXPOSE/CMD. Ensure any package installation, apt-get, corepack/pnpm install, and build steps that require root run before creating/switching the user; after those RUN steps, change ownership of /app (or relevant dirs) to appuser and use USER to run the container process so pnpm --filter "@mf-dashboard/web" start runs as the non-root user.apps/web/src/app/api/refresh/route.ts (1)
12-15: Consider using timing-safe comparison for token validation.While this is an internal service-to-service endpoint, using
crypto.timingSafeEqualprevents timing attacks and is a security best practice for secret comparison.🛡️ Proposed fix for timing-safe comparison
+import { timingSafeEqual } from "node:crypto"; import { revalidatePath } from "next/cache"; import { NextResponse } from "next/server"; export const dynamic = "force-dynamic"; +function safeCompare(a: string, b: string): boolean { + if (a.length !== b.length) return false; + return timingSafeEqual(Buffer.from(a), Buffer.from(b)); +} + export async function POST(request: Request) { const expected = process.env.REFRESH_TOKEN; if (!expected) { return NextResponse.json({ error: "refresh disabled" }, { status: 503 }); } const auth = request.headers.get("authorization"); - if (auth !== `Bearer ${expected}`) { + const expectedAuth = `Bearer ${expected}`; + if (!auth || !safeCompare(auth, expectedAuth)) { return NextResponse.json({ error: "unauthorized" }, { status: 401 }); } revalidatePath("/", "layout"); return NextResponse.json({ revalidated: true }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/api/refresh/route.ts` around lines 12 - 15, The authorization header comparison using auth !== `Bearer ${expected}` should be replaced with a timing-safe comparison: ensure you extract the token value from auth (handle missing auth by returning NextResponse.json({ error: "unauthorized" }, { status: 401 })), then convert both the provided token and the expected token to Buffer (or Uint8Array), verify lengths match, and use crypto.timingSafeEqual to compare them; if lengths differ or timingSafeEqual returns false, return the same 401 response. Update the logic around the auth and expected variables in route.ts to use this approach so comparisons are constant-time.compose.yml (1)
16-17: Pin thecloudflaredimage version for reproducibility.Using
latesttag can lead to unexpected behavior when Cloudflare pushes updates. Pin to a specific version for more predictable deployments.♻️ Proposed fix
cloudflared: - image: cloudflare/cloudflared:latest + image: cloudflare/cloudflared:2026.3.0 command: tunnel --no-autoupdate run🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@compose.yml` around lines 16 - 17, The compose file pins the cloudflared service to the mutable image tag "cloudflare/cloudflared:latest"; change the image specification for the cloudflared service (service name "cloudflared" and its image field) to a specific, tested version (e.g., "cloudflare/cloudflared:<semver>" or a digest) to ensure reproducible deployments and stability—replace "latest" with the chosen fixed version and update any deployment docs or CI that rely on the tag.README.md (1)
57-57: Consider adding authentication detail to the diagram.The diagram shows
POST /api/refresh(line 57) but doesn't indicate that it requires Bearer authentication. While line 69 mentions "Bearer 認証付き POST" in the prose, making this visible in the diagram would improve clarity about the security boundary between the crawler and web services.💡 Optional diagram enhancement
- B -->|7. POST /api/refresh| W + B -->|7. POST /api/refresh<br/>(Bearer token)| W🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 57, Update the diagram to show that the POST /api/refresh interaction requires Bearer authentication: annotate the arrow or label for the step that currently reads "POST /api/refresh" (the diagram element referenced by "W -->|8. localhost:8765| H[cloudflared コンテナ]" / the POST /api/refresh label) with "Bearer 認証" or similar, and optionally add a lock/icon on the target node to indicate the security boundary so it matches the prose "Bearer 認証付き POST".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/app/api/refresh/route.ts`:
- Around line 6-19: Add unit tests for the POST handler to cover the three paths
in apps/web/src/app/api/refresh/route.ts: when process.env.REFRESH_TOKEN is
undefined (expect 503 and JSON {error: "refresh disabled"}), when an invalid or
missing Authorization header is sent (set REFRESH_TOKEN and expect 401 and JSON
{error: "unauthorized"}), and when a valid Bearer token is provided (set
REFRESH_TOKEN, mock revalidatePath to verify it is called with "/" and "layout",
call POST with the Authorization header and assert a successful JSON response
{revalidated: true} and correct status). In tests, import the POST function,
stub/mock revalidatePath, set and restore process.env.REFRESH_TOKEN per test,
and check response body and status for each scenario.
---
Outside diff comments:
In `@README.md`:
- Around line 10-11: Update the README paragraph that describes using GitHub
Actions crontab: remove or replace the description about scheduling via GitHub
workflow and crontab and clearly state that scheduling is now performed inside
the crawler container using supercronic under Docker Compose; mention the
configured schedules (daily 06:50 JST and 15:20 JST with the -10 minute
adjustment) and point readers to the Docker Compose setup and the supercronic
cron file (referencing "crawler" container and "supercronic") so it's clear
where to change timing going forward.
---
Nitpick comments:
In `@apps/web/src/app/api/refresh/route.ts`:
- Around line 12-15: The authorization header comparison using auth !== `Bearer
${expected}` should be replaced with a timing-safe comparison: ensure you
extract the token value from auth (handle missing auth by returning
NextResponse.json({ error: "unauthorized" }, { status: 401 })), then convert
both the provided token and the expected token to Buffer (or Uint8Array), verify
lengths match, and use crypto.timingSafeEqual to compare them; if lengths differ
or timingSafeEqual returns false, return the same 401 response. Update the logic
around the auth and expected variables in route.ts to use this approach so
comparisons are constant-time.
In `@compose.yml`:
- Around line 16-17: The compose file pins the cloudflared service to the
mutable image tag "cloudflare/cloudflared:latest"; change the image
specification for the cloudflared service (service name "cloudflared" and its
image field) to a specific, tested version (e.g.,
"cloudflare/cloudflared:<semver>" or a digest) to ensure reproducible
deployments and stability—replace "latest" with the chosen fixed version and
update any deployment docs or CI that rely on the tag.
In `@docker/crawler/Dockerfile`:
- Around line 24-29: The Dockerfile currently runs COPY . . and then redundantly
re-COPYs docker/crawler/crontab, docker/crawler/entrypoint.sh, and
docker/crawler/run-crawl.sh; remove those extra COPY lines and keep only the RUN
chmod +x /app/docker/crawler/entrypoint.sh /app/docker/crawler/run-crawl.sh to
set executables (ensure the chmod paths match where COPY . . places files, e.g.,
/app/docker/crawler/* or adjust the chmod target if the context places them
elsewhere).
- Around line 1-31: Add a non-root user and switch to it after files and
binaries are installed: create a user/group (e.g., appuser) and a writable app
directory, chown /app and any installed binaries like /usr/local/bin/supercronic
to that user after the RUN steps that install packages and copy files, then add
a USER appuser (and set HOME if needed) before the ENTRYPOINT so ENTRYPOINT
["/usr/bin/tini", "--", "/app/docker/crawler/entrypoint.sh"] runs as the
non-root user; ensure the existing chmod lines for
/app/docker/crawler/entrypoint.sh and run-crawl.sh still apply or are performed
as root before changing to USER.
In `@docker/web/Dockerfile`:
- Around line 1-27: Add a non-root user and switch to it after any root-only
setup: create a dedicated user/group (e.g., appuser), chown the /app workspace
to that user, and add USER appuser before the final EXPOSE/CMD. Ensure any
package installation, apt-get, corepack/pnpm install, and build steps that
require root run before creating/switching the user; after those RUN steps,
change ownership of /app (or relevant dirs) to appuser and use USER to run the
container process so pnpm --filter "@mf-dashboard/web" start runs as the
non-root user.
In `@README.md`:
- Line 57: Update the diagram to show that the POST /api/refresh interaction
requires Bearer authentication: annotate the arrow or label for the step that
currently reads "POST /api/refresh" (the diagram element referenced by "W -->|8.
localhost:8765| H[cloudflared コンテナ]" / the POST /api/refresh label) with "Bearer
認証" or similar, and optionally add a lock/icon on the target node to indicate
the security boundary so it matches the prose "Bearer 認証付き POST".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: afdda501-ade2-493c-8c1b-2265a5d5f212
📒 Files selected for processing (18)
.dockerignore.env.example.github/workflows/daily-update.yml.github/workflows/deploy-demo.ymlREADME.mdapps/web/next.config.tsapps/web/src/app/api/refresh/route.tscompose.ymldocker/crawler/Dockerfiledocker/crawler/crontabdocker/crawler/entrypoint.shdocker/crawler/run-crawl.shdocker/web/Dockerfiledocs/setup.mdterraform/.env.templateterraform/README.mdterraform/variables.tfterraform/versions.tf
💤 Files with no reviewable changes (2)
- .github/workflows/daily-update.yml
- .github/workflows/deploy-demo.yml
✅ Files skipped from review due to trivial changes (7)
- terraform/.env.template
- .env.example
- .dockerignore
- docker/crawler/crontab
- terraform/versions.tf
- terraform/variables.tf
- terraform/README.md
| export async function POST(request: Request) { | ||
| const expected = process.env.REFRESH_TOKEN; | ||
| if (!expected) { | ||
| return NextResponse.json({ error: "refresh disabled" }, { status: 503 }); | ||
| } | ||
|
|
||
| const auth = request.headers.get("authorization"); | ||
| if (auth !== `Bearer ${expected}`) { | ||
| return NextResponse.json({ error: "unauthorized" }, { status: 401 }); | ||
| } | ||
|
|
||
| revalidatePath("/", "layout"); | ||
| return NextResponse.json({ revalidated: true }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for existing tests for the refresh route
rg -l 'refresh' --glob '*test*' --glob '*.spec.*' apps/web/
fd -e test.ts -e spec.ts . apps/web/src/app/api/Repository: hiroppy/mf-dashboard
Length of output: 46
🏁 Script executed:
# Check test file structure in apps/web to understand naming conventions
find apps/web -type f \( -name "*.test.ts" -o -name "*.spec.ts" -o -name "*.test.tsx" -o -name "*.spec.tsx" \) | head -20Repository: hiroppy/mf-dashboard
Length of output: 875
🏁 Script executed:
# Check if refresh route file exists in git and if it's new
cd apps/web/src/app/api && git log --oneline refresh/route.ts 2>/dev/null || echo "File may be new or git tracking issue"Repository: hiroppy/mf-dashboard
Length of output: 125
🏁 Script executed:
# Check the overall structure of API routes
ls -la apps/web/src/app/api/ 2>/dev/null | head -20Repository: hiroppy/mf-dashboard
Length of output: 229
🏁 Script executed:
# Search for any test patterns related to API routes
rg 'describe.*route|test.*route' apps/web/src --type ts --type tsx -l | head -10Repository: hiroppy/mf-dashboard
Length of output: 92
🏁 Script executed:
# Check git diff to see if refresh route is new or modified
git diff HEAD -- apps/web/src/app/api/refresh/route.ts | head -50Repository: hiroppy/mf-dashboard
Length of output: 46
🏁 Script executed:
# Check git status
git status apps/web/src/app/api/refresh/route.tsRepository: hiroppy/mf-dashboard
Length of output: 130
🏁 Script executed:
# Look for any testing setup for API routes or middleware
find apps/web -type d -name "__tests__" -o -type d -name "test" | head -10Repository: hiroppy/mf-dashboard
Length of output: 46
🏁 Script executed:
# Check if there's a vitest or jest config
find apps/web -name "vitest.config.*" -o -name "jest.config.*" -o -name "*.config.ts" | grep -E "test|vitest|jest"Repository: hiroppy/mf-dashboard
Length of output: 90
🏁 Script executed:
# Check what files are actually part of this PR/review
git diff --name-only HEAD~1 HEAD | head -20Repository: hiroppy/mf-dashboard
Length of output: 251
🏁 Script executed:
# If that doesn't work, check the current branch/staging
git diff --name-only --cachedRepository: hiroppy/mf-dashboard
Length of output: 46
🏁 Script executed:
# Check the overall git log to understand context
git log --oneline -10Repository: hiroppy/mf-dashboard
Length of output: 125
🏁 Script executed:
# List what's in the refresh directory
ls -la apps/web/src/app/api/refresh/Repository: hiroppy/mf-dashboard
Length of output: 230
🏁 Script executed:
# Read the actual route.ts file to confirm it contains new logic
cat apps/web/src/app/api/refresh/route.tsRepository: hiroppy/mf-dashboard
Length of output: 647
🏁 Script executed:
# Check if there's a force-dynamic directive
head -10 apps/web/src/app/api/refresh/route.tsRepository: hiroppy/mf-dashboard
Length of output: 389
Add unit tests for this API route handler.
Per coding guidelines (**/*.{ts,tsx}: New logic MUST have unit tests), this route with authentication logic and error paths requires unit test coverage. Tests should cover the disabled state (503), unauthorized access (401), and successful revalidation scenarios.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/app/api/refresh/route.ts` around lines 6 - 19, Add unit tests
for the POST handler to cover the three paths in
apps/web/src/app/api/refresh/route.ts: when process.env.REFRESH_TOKEN is
undefined (expect 503 and JSON {error: "refresh disabled"}), when an invalid or
missing Authorization header is sent (set REFRESH_TOKEN and expect 401 and JSON
{error: "unauthorized"}), and when a valid Bearer token is provided (set
REFRESH_TOKEN, mock revalidatePath to verify it is called with "/" and "layout",
call POST with the Authorization header and assert a successful JSON response
{revalidated: true} and correct status). In tests, import the POST function,
stub/mock revalidatePath, set and restore process.env.REFRESH_TOKEN per test,
and check response body and status for each scenario.
f1ad730 to
f0076bf
Compare
DB を Git に commit する運用をやめ、ローカル PC の Docker Compose で
web (Next.js) / cloudflared / crawler の 3 サービスを常駐させる構成へ
移行する。crawler コンテナは supercronic で内部 cron を回し、JST 6:50/
15:20 に Playwright で MoneyForward をスクレイピング → 完了後 web の
/api/refresh を Bearer 認証で叩いて revalidatePath('/', 'layout') で
SSG 全ルートを invalidate する。
Cloudflare 側 (Tunnel / DNS CNAME / Access Application + Email
allowlist Policy) は terraform/ 配下に Provider v5 で宣言的に記述。
主な変更:
- compose.yml / .dockerignore / .env.example を新設し 3 サービスを定義
- docker/web/Dockerfile: node:22-bookworm-slim + pnpm install で
workspace を絞ってインストール、data/demo.db を placeholder に
next build をイメージに焼く
- docker/crawler/Dockerfile: mcr.microsoft.com/playwright:v1.58.2-jammy
+ supercronic + tini。entrypoint で初回 crawl の有無を確認
- apps/web/src/app/api/refresh/route.ts: REFRESH_TOKEN の Bearer 認証で
revalidatePath を呼ぶ
- apps/web/next.config.ts: output: \"export\" は GITHUB_PAGES=true の
時のみ条件付け (本番は SSG + next start)
- terraform/ 一式: Tunnel + DNS + Access (Google IdP + email allowlist)
- wrangler.toml / .github/workflows/daily-update.yml を削除
- .github/workflows/deploy-demo.yml の private-repo 判別 check job を削除
- .gitignore に data/moneyforward.db を追加
- docs/setup.md と README.md を Docker Compose 運用に全面書き換え
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
f0076bf to
2b7e58c
Compare
CLAUDE.md の "新規ロジックは unit test 必須" に従い、refresh route の
3 経路 (REFRESH_TOKEN 未設定で 503 / Bearer 不一致で 401 / 一致で
revalidatePath('/', 'layout') が呼ばれて 200) をカバー。
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
web は expose: のみで host に公開しておらず、外側は cloudflared + Cloudflare Access (Google IdP + email allowlist) で gate されている。 /api/refresh に到達できるのは crawler コンテナ (Docker bridge 内、 信頼済み) と Access 通過済みユーザーのみなので、Bearer 認証は 冗長な二重防御。コードと .env / docs から REFRESH_TOKEN を撤去。 - apps/web/src/app/api/refresh/route.ts: 認証分岐を削除し POST で 直接 revalidatePath を呼ぶ - apps/web/src/app/api/refresh/route.test.ts: 1 ケース (POST -> 200 + revalidatePath が '/' 'layout' で呼ばれる) に簡素化 - docker/crawler/run-crawl.sh: Authorization ヘッダ削除 (リトライは web 起動待ちのため残す) - .env.example / docs/setup.md / README.md: REFRESH_TOKEN 関連を撤去、 内部前提を明示 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
run-crawl.sh の責務 (crawl -> POST /api/refresh) を crawler 本体に 取り込み、shell wrapper と Authorization ヘッダ運用を撤去。 - apps/crawler/src/web-refresh.ts (新規): WEB_URL が設定されていれば 最大 12 回 (5 秒間隔) リトライで /api/refresh を POST。slack/discord と同じ通知モジュールパターン。WEB_URL 未設定時はスキップなのでローカル 実行を壊さない - apps/crawler/src/index.ts: 既存の slack/discord 通知の直後に notifyWebRefresh() を呼ぶ (失敗時は error ログのみで crawl 全体は 落とさない) - docker/crawler/run-crawl.sh: 削除 - docker/crawler/crontab: pnpm --filter @mf-dashboard/crawler start を 直接起動 - docker/crawler/entrypoint.sh: 初回 crawl も同じコマンドで実行 - docker/crawler/Dockerfile: run-crawl.sh への chmod を削除 - docs/setup.md: 手動コマンドを pnpm 直叩きに更新 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ENTRYPOINT を tini -- supercronic /app/docker/crawler/crontab に直結し、 初回 DB ブートストラップ用の薄いシェル wrapper を撤去。 初回 DB は docs に記載した手動コマンド (docker compose exec crawler pnpm --filter @mf-dashboard/crawler start) を 1 回だけ走らせる前提。 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
apps/web/out 直配信や scripts/start-tunnel.sh などの撤去済み記述を、 Docker Compose 構成と .env の TUNNEL_TOKEN 経由の運用に書き換える。 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
entrypoint.sh の削除と整合させる。 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
terraform/.env.template を撤去し、Docker Compose と Terraform の 両方が root .env を参照する構成にする。CLOUDFLARE_API_TOKEN は op:// 参照のまま .env に記述し、op run が terraform 実行時に解決する。 コマンドはすべて root から `op run --env-file=.env -- terraform -chdir=terraform <subcommand>` 形式に統一。 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Summary
web(Next.js) /cloudflared/crawlerの 3 サービスを常駐させる構成に切り替え/api/refreshに Bearer 認証で POST →revalidatePath('/', 'layout')で SSG 全ルートを invalidateサービス構成
webnext start --port 8765)。image build 時にdata/demo.dbで bootstrap 済み、実 DB は volume 経由で読むnode:22-bookworm-slimcloudflaredTUNNEL_TOKENで Cloudflare Edge に接続cloudflare/cloudflared:latestcrawlerdocker/crawler/crontabのスケジュールで実行し、終了後 web の/api/refreshを叩くmcr.microsoft.com/playwright:v1.58.2-jammyすべて
restart: unless-stopped+./dataを bind mount。主な変更
compose.yml,.dockerignore,.env.example(新規)docker/web/Dockerfile(新規)pnpm install --filter "@mf-dashboard/web..."→data/demo.dbを使ったnext buildをイメージに焼くdocker/crawler/{Dockerfile,crontab,entrypoint.sh,run-crawl.sh}(新規)web:8765/api/refreshを叩くapps/web/src/app/api/refresh/route.ts(新規)REFRESH_TOKENの Bearer 認証でrevalidatePath('/', 'layout')を呼ぶterraform/(新規)apps/web/next.config.tsoutput: "export"をGITHUB_PAGES=trueの時のみ条件付け (本番は SSG +next start)apps/web/src/instrumentation.tsapps/web/package.jsonnode-cron/@types/node-cronを削除scripts/start-web.sh/scripts/start-tunnel.shpackage.jsonstart:web/tunnelscripts を削除wrangler.toml.gitignoredata/moneyforward.dbを追加.github/workflows/daily-update.yml.github/workflows/deploy-demo.ymldocs/setup.md/README.mdTest plan
terraform fmt -check/terraform validate(Provider v5.19.0)pnpm --filter @mf-dashboard/web typecheckdocker compose config --quiet(.env.exampleをコピーした状態で OK)docker compose build web成功 —apps/web/.next/server/app/api/refresh/route.jsが生成されることを確認curl http://localhost:8765/HTTP 200/api/refresh/に Bearer なし / wrong token → 401、正しいREFRESH_TOKEN→{"revalidated":true}terraform apply(account_id/zone_id/hostname/allowed_emailsと Cloudflare API Token が必要)terraform output -raw tunnel_tokenを.envのTUNNEL_TOKENに書き込みdocker compose up -dで 3 サービス起動 →https://<hostname>/で Google ログイン → ダッシュボード表示/api/refresh経由で revalidate されることをログで確認🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
Chores