diff --git a/.github/workflows/lint-test.yml b/.github/workflows/lint-test.yml index f51feb3..6f5a0b6 100644 --- a/.github/workflows/lint-test.yml +++ b/.github/workflows/lint-test.yml @@ -53,43 +53,59 @@ jobs: env: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} - integration-test-latest: + integration-test-labels: needs: [run] permissions: contents: read runs-on: ubuntu-24.04 + strategy: + fail-fast: false + matrix: + traefik: [v3.6, v3.7, latest] + env: + TRAEFIK_TAG: ${{ matrix.traefik }} steps: - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7 - name: run run: go run test.go working-directory: ./ci - env: - TRAEFIK_TAG: latest + + - name: capture logs + if: ${{ always() }} + run: docker compose logs --tail 100 nginx nginx2 traefik + working-directory: ./ci - name: cleanup if: ${{ always() }} - run: docker compose logs --tail 100 nginx nginx2 traefik && docker compose down + run: docker compose down working-directory: ./ci - integration-test: - needs: [integration-test-latest] + integration-test-multilayer-routing: + needs: [run] permissions: contents: read runs-on: ubuntu-24.04 strategy: + fail-fast: false matrix: - traefik: [v2.11, v3.0, v3.1, v3.2, v3.3, v3.4, v3.5, v3.6, v3.7] + traefik: [v3.6, v3.7, latest] + env: + COMPOSE_FILE: docker-compose.multilayer.yml + TRAEFIK_TAG: ${{ matrix.traefik }} steps: - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7 - name: run run: go run test.go working-directory: ./ci - env: - TRAEFIK_TAG: ${{ matrix.traefik }} + + - name: capture logs + if: ${{ always() }} + run: docker compose logs --tail 100 nginx nginx2 traefik + working-directory: ./ci - name: cleanup if: ${{ always() }} - run: docker compose logs --tail 100 nginx nginx2 traefik && docker compose down + run: docker compose down working-directory: ./ci diff --git a/.github/workflows/stress-test.yaml b/.github/workflows/stress-test.yaml deleted file mode 100644 index 2c12475..0000000 --- a/.github/workflows/stress-test.yaml +++ /dev/null @@ -1,60 +0,0 @@ -name: stress-test - -on: - pull_request: - -jobs: - stress-test: - permissions: - contents: read - pull-requests: write - runs-on: ubuntu-24.04 - steps: - - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7 - - - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6 - with: - go-version: "1.26.4" - - - name: Run stress tests - id: stress_test - run: ./ci/run-stress-tests.sh > stress_results.md - - - name: Comment PR with stress test results - uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9 - with: - script: | - const fs = require('fs'); - const results = fs.readFileSync('stress_results.md', 'utf8'); - - // Find existing comment - const { data: comments } = await github.rest.issues.listComments({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: context.issue.number, - }); - - const botComment = comments.find(comment => - comment.user.type === 'Bot' && - comment.body.includes('Running State Persistence Stress Tests') - ); - - const commentBody = results; - - if (botComment) { - // Update existing comment - await github.rest.issues.updateComment({ - owner: context.repo.owner, - repo: context.repo.repo, - comment_id: botComment.id, - body: commentBody - }); - } else { - // Create new comment - await github.rest.issues.createComment({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: context.issue.number, - body: commentBody - }); - } diff --git a/.traefik.yml b/.traefik.yml index 3cd0a51..f4e3a27 100644 --- a/.traefik.yml +++ b/.traefik.yml @@ -1,14 +1,12 @@ displayName: Captcha Protect -summary: Add an anti-bot challenge to individual IPs in a subnet when traffic spikes are detected from that subnet +summary: Add an anti-bot challenge to protected routes type: middleware import: github.com/libops/captcha-protect testData: ProtectRoutes: / ChallengeTmpl: challenge.tmpl.html Window: 86400 - RateLimit: 20 IPForwardedHeader: "X-Forwarded-For" CaptchaProvider: turnstile SiteKey: 1x00000000000000000000AA SecretKey: 1x0000000000000000000000000000000AA - EnableStateReconciliation: "false" diff --git a/README.md b/README.md index 239ba5c..f626125 100644 --- a/README.md +++ b/README.md @@ -1,330 +1,28 @@ # Captcha Protect + [![lint-test](https://github.com/libops/captcha-protect/actions/workflows/lint-test.yml/badge.svg)](https://github.com/libops/captcha-protect/actions/workflows/lint-test.yml) [![Go Report Card](https://goreportcard.com/badge/github.com/libops/captcha-protect)](https://goreportcard.com/report/github.com/libops/captcha-protect) [![codecov](https://codecov.io/gh/libops/captcha-protect/branch/main/graph/badge.svg)](https://codecov.io/gh/libops/captcha-protect) -Traefik middleware to challenge individual IPs in a subnet when traffic spikes are detected from that subnet, using a captcha of your choice for the challenge (turnstile, recaptcha, or hcaptcha). **Requires traefik `v2.11.1` or above** - -You may have seen CAPTCHAs added to individual forms on the web to prevent bots from spamming submissions. This plugin extends that concept to your entire site (or specific routes on your site), effectively placing your entire site behind a CAPTCHA. However, the CAPTCHA is only triggered when a spike in traffic is detected from the same IP subnet. Once the CAPTCHA is successfully completed, that IP is no longer challenged, allowing uninterrupted browsing. - -
anti-bot decision tree - -```mermaid -flowchart TD - Client(Client accesses path on website) --> IP{Has client passed captcha challenge in the last 24h?} - IP -- Yes --> Continue(Go to original destination) - IP -- No --> IP_BYPASS{Is client IP excluded by captcha-protect config?} - IP_BYPASS -- Yes --> Continue(Go to original destination) - IP_BYPASS -- No --> GOOD_BOT{Is client IP hostname in allowed bot list?} - GOOD_BOT -- No --> PROTECTED_ROUTE{Is this route protected?} - GOOD_BOT -- Yes --> CANONICAL_URL_BOT{Are there URL parameters?} - CANONICAL_URL_BOT -- Yes --> PROTECTED_ROUTE{Is this route prefix in protectRoutes?} - CANONICAL_URL_BOT -- No --> Continue(Go to original destination) - PROTECTED_ROUTE -- Yes --> RATE_LIMIT{Is this IP in a range seeing increased traffic?} - PROTECTED_ROUTE -- No --> Continue(Go to original destination) - RATE_LIMIT -- Yes --> REDIRECT(Redirect to /challenge) - RATE_LIMIT -- No --> Continue(Go to original destination) - REDIRECT --> CIRCUIT{Is circuit breaker open?} - CIRCUIT -- Yes --> POJ_CHALLENGE{Proof-of-Javascript challenge} - CIRCUIT -- No --> CAPTCHA_CHALLENGE{turnstile/recaptcha/hcaptcha challenge} - POJ_CHALLENGE -- Pass --> Continue(Go to original destination) - POJ_CHALLENGE -- Fail --> Stuck - CAPTCHA_CHALLENGE -- Pass --> Continue(Go to original destination) - CAPTCHA_CHALLENGE -- Fail --> Stuck -``` -
- -## Config - -### Example - -Below is an example `docker-compose.yml` with traefik as the frontend, and nginx as the backend. nginx is adding the captcha-protect plugin as middleware to protect routes on the site that start with `/` (`protectRoutes: "/"`) - -Since these config values are specified: - -``` -rateLimit: 0 -window: 864000 -``` - -Any individual IP would immediately be challenged. Once the client at that IP passes a challenge they won't be challenged again for 10 days (86400**0**). This is the recommended default given the wide network range of some abusive crawlers. - -```yaml -networks: - default: -services: - nginx: - image: nginx:${NGINX_TAG} - labels: - traefik.enable: true - traefik.http.routers.nginx.entrypoints: http - traefik.http.routers.nginx.service: nginx - traefik.http.routers.nginx.rule: Host(`${DOMAIN}`) - traefik.http.services.nginx.loadbalancer.server.port: 80 - traefik.http.routers.nginx.middlewares: captcha-protect@docker - traefik.http.middlewares.captcha-protect.plugin.captcha-protect.rateLimit: 0 - traefik.http.middlewares.captcha-protect.plugin.captcha-protect.ipv4subnetMask: 8 - traefik.http.middlewares.captcha-protect.plugin.captcha-protect.window: 864000 - traefik.http.middlewares.captcha-protect.plugin.captcha-protect.protectRoutes: "/" - traefik.http.middlewares.captcha-protect.plugin.captcha-protect.captchaProvider: turnstile - traefik.http.middlewares.captcha-protect.plugin.captcha-protect.siteKey: ${TURNSTILE_SITE_KEY} - traefik.http.middlewares.captcha-protect.plugin.captcha-protect.secretKey: ${TURNSTILE_SECRET_KEY} - traefik.http.middlewares.captcha-protect.plugin.captcha-protect.goodBots: apple.com,archive.org,commoncrawl.org,duckduckgo.com,facebook.com,instagram.com,kagibot.org,linkedin.com,msn.com,openalex.org,twitter.com,x.com - traefik.http.middlewares.captcha-protect.plugin.captcha-protect.persistentStateFile: /tmp/state.json - traefik.http.middlewares.captcha-protect.plugin.captcha-protect.enableStateReconciliation: "false" - traefik.http.middlewares.captcha-protect.plugin.captcha-protect.enableGooglebotIPCheck: "true" - traefik.http.middlewares.captcha-protect.plugin.captcha-protect.enableUptimeRobotBypass: "false" - traefik.http.middlewares.captcha-protect.plugin.captcha-protect.periodSeconds: 30 - traefik.http.middlewares.captcha-protect.plugin.captcha-protect.failureThreshold: 3 - networks: - default: - aliases: - - nginx - traefik: - image: traefik:${TRAEFIK_TAG} - command: >- - --api.insecure=false - --api.dashboard=false - --api.debug=false - --ping=true - --entryPoints.http.address=:80 - --providers.docker=true - --providers.docker.network=default - --experimental.plugins.captcha-protect.modulename=github.com/libops/captcha-protect - --experimental.plugins.captcha-protect.version=v1.13.1 - volumes: - - /var/run/docker.sock:/var/run/docker.sock:z - - /CHANGEME/TO/A/HOST/PATH/FOR/STATE/FILE:/tmp/state.json:rw - ports: - - "80:80" - networks: - default: - aliases: - - traefik - healthcheck: - test: traefik healthcheck --ping - depends_on: - nginx: - condition: service_started -``` - -### Protecting multiple services - -Define `captcha-protect` once and attach that same named middleware to every router that should use it. For example, if the full `traefik.http.middlewares.captcha-protect.*` configuration is on the `nginx` service above, another service only needs to reference it: - -```yaml -services: - nginx: - labels: - traefik.http.routers.nginx.middlewares: captcha-protect@docker - # Define all traefik.http.middlewares.captcha-protect.* labels here. - - another-service: - labels: - traefik.enable: true - traefik.http.routers.another-service.middlewares: captcha-protect@docker - # Do not repeat the middleware definition here. -``` - -If a router already uses other middleware, add `captcha-protect@docker` to its comma-separated middleware list. - -Traefik creates a separate plugin instance for each router, even when several routers reference the same named middleware. Each instance has its own in-memory rate, verification, and bot caches. This behavior is described in [issue #52](https://github.com/libops/captcha-protect/issues/52) and is why state reconciliation exists. - -Set `enableStateReconciliation: "true"` when the middleware is attached to more than one router and those instances use the same `persistentStateFile`. Also enable it for every separately named middleware definition or Traefik replica that writes to that file. All instances must see the same state file and its adjacent `.lock` file. Reconciliation synchronizes file-backed rate and verified state so that a challenge completed through one router is recognized by the others. - -#### Memory considerations - -Reconciliation does not turn the per-router caches into shared memory. Steady-state cache memory therefore grows approximately with the number of routers: two router instances holding the same state use roughly twice the cache memory. Bot lookup cache entries are also duplicated per router and are not persisted. - -The cache implementation is expiration-based; despite its `lru` import alias in the source, it does not enforce a least-recently-used entry limit, maximum entry count, or memory limit. Enough distinct client IPs and subnets within the configured `window` can therefore consume substantial memory. A longer `window` retains entries longer. More-specific subnet masks, such as a larger `ipv4subnetMask`, can create more distinct rate-cache entries for the same traffic. `rateLimit` controls when a subnet is challenged; it does not cap the number of cached subnets. Verified-client and bot caches are keyed by individual IP. Account for all protected routers when sizing Traefik memory and monitor process memory under representative traffic. See [issue #95](https://github.com/libops/captcha-protect/issues/95) for the discussion that prompted this guidance. - -Reconciliation temporarily needs additional memory to read and decode the complete state file and to snapshot cache entries, so peak memory is higher than the steady-state cache size. The `memory` values in `state.json` estimate one copy of the persisted rate and verified caches only; they exclude bot-cache entries, Go runtime and map overhead, other plugin instances, and temporary reconciliation allocations. They do not represent total process or peak memory. Avoid reconciliation for state approaching one million verified visitors: it can take 5-8 seconds at that scale and competes with the 10-second save interval. - -There is currently no shared in-process or external cache backend that removes this per-router duplication. For installations with many sites, an alternative is to route them through one lightweight proxy service and attach `captcha-protect` only to that proxy's router. The proxy can then route accepted requests to the respective backend services. This reduces the number of captcha-protect instances and cache copies, but makes the proxy part of the request path and requires it to preserve the original client IP correctly. - -### Config options - -| **Parameter** | **Type (Required)** | **Default** | **Description** | -|-------------------------|-------------------------|--------------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| `mode` | `string` | `prefix` | Must be: `prefix`, `suffix`, `regex`. Matching does not include query parameters. `excludeRoutes` always uses `prefix` except when `mode: regex`. Only use `regex` when needed | -| `protectRoutes` | `[]string` (required) | `""` | Comma-separated list of route prefixes/suffixes/regex patterns to protect. | -| `excludeRoutes` | `[]string` | `""` | Comma-separated list of route prefixes to **never** protect. e.g., `protectRoutes: "/"` protects the entire site. `excludeRoutes: "/ajax"` would never challenge any route starting with `/ajax` | -| `captchaProvider` | `string` (required) | `""` | The captcha type to use. Supported values: `turnstile`, `hcaptcha`, `recaptcha`, and `poj` (proof-of-javascript). | -| `siteKey` | `string` (required) | `""` | The captcha site key. | -| `secretKey` | `string` (required) | `""` | The captcha secret key. | -| `periodSeconds` | `int` | `0` | Health check interval (in seconds) for the primary captcha provider. The circuit breaker uses this to detect provider outages. | -| `failureThreshold` | `int` | `0` | Number of consecutive health check failures before the circuit breaker opens and switches to proof-of-javascript fallback. | -| `rateLimit` | `uint` | `20` | Maximum requests allowed from a subnet before a challenge is triggered. | -| `window` | `int` | `86400` | Duration (in seconds) for monitoring requests per subnet and retaining cache entries. Longer windows can increase memory use. | -| `ipv4subnetMask` | `int` | `16` | CIDR subnet mask to group IPv4 addresses for rate limiting. More-specific (larger) masks can create more rate-cache entries and increase memory use. | -| `ipv6subnetMask` | `int` | `64` | CIDR subnet mask to group IPv6 addresses for rate limiting. More-specific (larger) masks can create more rate-cache entries and increase memory use. | -| `ipForwardedHeader` | `string` | `""` | Header to check for the original client IP if Traefik is behind a load balancer. Only set this when Traefik receives the header from a trusted proxy/load balancer; otherwise clients can spoof their IP. | -| `ipDepth` | `int` | `0` | How deep past the last non-exempt IP to fetch the real IP from `ipForwardedHeader`. Default 0 returns the last IP in the forward header | -| `goodBots` | `[]string` (encouraged) | *see below* | List of second-level domains for bots that are never challenged or rate-limited. | -| `enableGooglebotIPCheck`| `string`. | `"false"` | Treat IPs coming from googlebot's known IP ranges as good bots | -| `enableUptimeRobotBypass` | `string` | `"false"` | When `"true"`, bypass challenges for IP ranges published by UptimeRobot. The ranges are refreshed every 24 hours. | -| `protectParameters` | `string` | `"false"` | Forces rate limiting even for good bots if URL parameters are present. Useful for protecting faceted search pages. | -| `protectFileExtensions` | `[]string` | `""` | Comma-separated file extensions to protect. By default, your protected routes only protect html files. This is to prevent files like CSS/JS/img from tripping the rate limit. | -| `protectHttpMethods` | `[]string` | `"GET,HEAD"` | Comma-separated list of HTTP methods to protect against | -| `exemptIps` | `[]string` | `privateIPs` | CIDR-formatted IPs that should never be challenged. Private IP ranges are always exempt. | -| `exemptUserAgents` | `[]string` | `""` | Comma-separated list of case-insensitive user agent substrings to never challenge. For example, `exemptUserAgents: YandexBot` exempts user agents containing `YandexBot`. | -| `challengeURL` | `string` | `"/challenge"` | URL where challenges are served. This will override existing routes if there is a conflict. Setting to blank will have the challenge presented on the same page that tripped the rate limit. | -| `challengeTmpl` | `string` | `"./challenge.tmpl.html"`| Path to the Go HTML template for the captcha challenge page. | -| `challengeStatusCode` | `int` | `200` | HTTP Response status code to return when serving a challenge | -| `enableStatsPage` | `string` | `"false"` | Allows `exemptIps` to access `/captcha-protect/stats` to monitor the rate limiter. | -| `logLevel` | `string` | `"INFO"` | Log level for the middleware. Options: `ERROR`, `WARNING`, `INFO`, or `DEBUG`. | -| `persistentStateFile` | `string` | `""` | File path to persist rate limiter and verified challenge state across Traefik restarts. When unset, no state load/save goroutine is started. Dirty local state is saved about every 60s plus 0-2s jitter. Derived bot lookup cache entries are not persisted. In Docker, mount this file from the host. | -| `enableStateReconciliation` | `string` | `"false"` | When `"true"`, polls the shared state file for changes and merges newer disk state into memory, then reconciles again before dirty snapshots are saved. Enable when more than one router, named middleware definition, or Traefik replica writes to the same state file. Traefik creates a separate plugin instance and in-memory cache per router, even when routers reuse one named middleware. Dirty shared state is saved about every 10s plus 0-2s jitter; see [Protecting multiple services](#protecting-multiple-services). | - -### Circuit Breaker (failover if a captcha provider is unavailable) - -The circuit breaker provides automatic failover when the primary captcha provider (Turnstile, reCAPTCHA, or hCaptcha) becomes unavailable. When enabled, it: - -1. **Enables a liveness probe on the captcha provider**: Periodically sends HEAD requests to the provider's JavaScript file (every `periodSeconds`). Also tracks 5xx errors during server side validation. -2. **Detects failures**: Counts consecutive health check failures -3. **Opens circuit**: After `failureThreshold` consecutive failures, switches to proof-of-javascript fallback -4. **Falls back to PoJ**: Ensures user is loading javascript. Requires revalidating in 1hr -5. **Auto-recovery**: Automatically returns to primary provider when health checks succeed - -**Proof-of-Javascript Fallback:** -- Requires browsers to submit a form -- Self-contained (no external dependencies) - -**Configuration:** -- Circuit breaker is enabled by setting two config settings: `periodSeconds` and `failureThreshold` - - e.g. `periodSeconds: 30` and `failureThreshold: 3` -- To disable: set both `periodSeconds: 0` and `failureThreshold: 0` (the default config) -- The `poj` provider can also be used directly as the primary provider (no circuit breaker needed) - -### Good Bots - -To avoid having this middleware impact your SEO score, it's recommended to provide a value for `goodBots`. By default, no bots will be allowed to crawl your protected routes beyond the rate limit unless their second level domain (e.g. `bing.com`) is configured as a good bot. - -A good default value for `goodBots` would be: - -``` -enableGooglebotIPCheck: "true" -enableUptimeRobotBypass: "true" -goodBots: apple.com,archive.org,duckduckgo.com,facebook.com,google.com,instagram.com,kagibot.org,linkedin.com,msn.com,openalex.org,twitter.com,x.com -``` - -Since google publishes their bot IPs, we can also leverage their API to let google crawl the site unchallenged based on client IP. This can be enabled with `enableGooglebotIPCheck: "true"` - -UptimeRobot publishes its monitoring IP ranges at `https://api.uptimerobot.com/meta/ips`. Set `enableUptimeRobotBypass: "true"` to exempt those IPs; the list is fetched at startup and refreshed every 24 hours. The default is `"false"`. - -**However** if you set the config parameter `protectParameters="true"`, even good bots won't be allowed to crawl protected routes if a URL parameter is on the request (e.g. `/foo?bar=baz`). This `protectParameters` feature is meant to help protect faceted search pages. - - -## Overriding the challenge template file - -You probably will want to theme the CAPTCHA challenge page to match the style of your site. - -You can do that by copying the [challenge.tmpl.html](./challenge.tmpl.html) file in this repo into your docker compose project, mounting it into your traefik container - -```yaml - traefik: - volumes: - - ./host/path/to/challenge.tmpl.html:/challenge.tmpl.html:ro -``` - -and pointing the middleware to your overridden template with - -```yaml - traefik.http.middlewares.captcha-protect.plugin.captcha-protect.challengeTmpl: "/challenge.tmpl.html" -``` - -When you override the challenge template, the process probably looks like: - -1. Copying some html file from your existing site (so the challenge looks like the rest of your site) -2. Replacing some `
` in the HTML body for the file copied in step 1 with the `
...
` HTML tags/contents in this repo's default [challenge.tmpl.html](./challenge.tmpl.html). You must copy the `form` and `script` tags exactly as they are in the original challenge template. They use go's templating language to inject the proper site key and other variables into the HTML response when a challenge is presented -3. You must also be sure to have this in the `` of your overridden template: - -``` - -``` - -## Similar projects - -- [Traefik RateLimit middleware](https://doc.traefik.io/traefik/middlewares/http/ratelimit/) - the core traefik ratelimit middleware will start sending 429 responses based on individual IPs, which might not be good enough to protect against traffic coming from distributed networks. Also, this plugin (captcha-protect) allows not including files in your rate limiter to avoid static assets from being counted in the rate limit. -- [crowdsec-bouncer-traefik-plugin](https://github.com/maxlerebourg/crowdsec-bouncer-traefik-plugin) has a captcha option, but requires integrating with crowdsec to verify individual IPs. This plugin (captcha-protect) instead just checks the traffic actually visiting your site and verifies the traffic is from a person only when the traffic exceeds some rate limit you configure. - -## Attribution - -- the original implementation of this logic was [a drupal module called turnstile_protect](https://www.drupal.org/project/turnstile_protect). This traefik plugin was made to make the challenge logic even more perfomant than that Drupal module, and also to provide this bot protection to non-Drupal websites -- making general captcha structs to support multiple providers was based on the work in [crowdsec-bouncer-traefik-plugin](https://github.com/maxlerebourg/crowdsec-bouncer-traefik-plugin) -- in memory cache thanks to https://github.com/patrickmn/go-cache - -## When to enable regex - -When possible, you want to keep regex disabled as seen in the example benchmark below. - -However, when needed it can be enabled with `mode: regex` - -``` -$ go mod init bench -$ cat << EOF > bench_test.go -package main - -import ( - "regexp" - "strings" - "testing" -) - -var ( - testPath = "/api/v1/user/profile" - prefix = "/api/v1" - regex = regexp.MustCompile("^/api/v1") -) - -func BenchmarkHasPrefix(b *testing.B) { - for i := 0; i < b.N; i++ { - _ = strings.HasPrefix(testPath, prefix) - } -} - -func BenchmarkRegexMatch(b *testing.B) { - for i := 0; i < b.N; i++ { - _ = regex.MatchString(testPath) - } -} -EOF -$ go test -bench=. -benchmem -``` - -``` -goos: darwin -goarch: amd64 -cpu: Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz -BenchmarkHasPrefix-12 340856451 3.415 ns/op 0 B/op 0 allocs/op -BenchmarkRegexMatch-12 27992568 41.20 ns/op 0 B/op 0 allocs/op -PASS -``` - - -## How to monitor the rate limiter +Captcha Protect is a Traefik middleware that challenges client IPs on protected routes. It can use Turnstile, reCAPTCHA, hCaptcha, or proof-of-javascript for the challenge. -If you set the `enableStatsPage` to true, it allows `exemptIps` to access /`captcha-protect/stats` to monitor the rate limiter. The key JSON key to look on the stats page is the top level "rate" key, which will list the subnets that are currently forced to be challenged based to request patterns and the `captcha-protect` configuration values used. +It requires Traefik `v3.6` or above. -If you have use a computer within the `exemptIps`, and access to the command line tools `curl` and `jq`, here is a recipe for how to list the top 25 subnets being challenged... +## Documentation -```bash -curl -s https://example.com/captcha-protect/stats | jq -r '.rate | to_entries | sort_by(.value) | .[] | "\(.key): \(.value)"' | tail -25 -``` +The user and operator documentation now lives at: -The rate limiter and verified challenge portions of this JSON state data are also found in the `state.json` file that you should have configured in your `docker-compose.yml` using the `persistentStateFile` setting and volume definition. When `enableStateReconciliation` is `"false"`, dirty state is saved roughly every 60 seconds plus 0-2 seconds of jitter. When `enableStateReconciliation` is `"true"` for multi-instance shared state, dirty state is saved roughly every 10 seconds plus 0-2 seconds of jitter. If `persistentStateFile` is unset, state persistence is disabled. NOTE: this file should only be changed by `captcha-protect` and not manually. + -## Troubleshooting +Start there for: -Here is a way to troubleshoot your `captcha-protect` set up. +- Docker Compose configuration examples. +- Full configuration option reference. +- Preferred multi-layer routing guidance for protecting multiple services. +- Challenge template customization. +- Good bot bypasses, monitoring, and troubleshooting. -### Verify that your Turnstile site-key is configured properly +## Source Links -One reason that may cause `captcha-protect` to not work is that the Cloudflare Turnstile widget site-key or private-key are not properly set for `captcha-protect` to access. Below is a way to confirm if the Turnstile site-key is configured correctly. (**WARNING**: There is currently no easy way to check the private-key, since the secret-key should never be displayed on a webpage or shared.) -1. Visit the `captcha-protect` "challenge" URL which is set to `https://example.com/challenge` by default. -1. You should see a web page that says "Verifying connection" -
**NOTE**: If you customized the `challengeTmpl` configuration, the page may say something different. -1. Look at the HTML source code for the page `https://example.com/challenge`, by right-clicking on the page and selecting "View page source" (on Chrome). -1. In the HTML source view that opens up, look for a `
` tag that has an attribute named `data-sitekey`, and check if its value matches your Cloudflare Turnstile widget sitekey value. -
**TIP**: You need to log in to your Cloudflare online account and go to the Turnstile section to see your site-key and secret-key values. -1. If the site-key value did not match in the HTML `
` tag, then update the `docker-compose.yml` and/or `.env` file to correctly pass the site-key value. Also check if the Cloudflare Turnstile widget secret-key is set correctly. +- Documentation source: +- Default challenge template: [challenge.tmpl.html](./challenge.tmpl.html) diff --git a/ci/.env b/ci/.env index a2cbe21..03c0513 100644 --- a/ci/.env +++ b/ci/.env @@ -1,4 +1,4 @@ -TRAEFIK_TAG=v3.5 +TRAEFIK_TAG=v3.6 NGINX_TAG=1.27.4-alpine3.21 TURNSTILE_SITE_KEY=1x00000000000000000000AA TURNSTILE_SECRET_KEY=1x0000000000000000000000000000000AA diff --git a/ci/conf/traefik/multilayer.yml b/ci/conf/traefik/multilayer.yml new file mode 100644 index 0000000..d8af677 --- /dev/null +++ b/ci/conf/traefik/multilayer.yml @@ -0,0 +1,54 @@ +http: + routers: + protected-parent: + entryPoints: + - http + rule: "Host(`localhost`)" + middlewares: + - captcha-protect + + nginx: + rule: "PathPrefix(`/`)" + priority: 1 + service: nginx + parentRefs: + - protected-parent + + nginx2: + rule: "PathPrefix(`/app2`)" + priority: 100 + service: nginx2 + parentRefs: + - protected-parent + + services: + nginx: + loadBalancer: + servers: + - url: "http://nginx:80" + nginx2: + loadBalancer: + servers: + - url: "http://nginx2:80" + + middlewares: + captcha-protect: + plugin: + captcha-protect: + captchaProvider: turnstile + window: 120 + siteKey: 1x00000000000000000000AA + secretKey: 1x0000000000000000000000000000000AA + enableStatsPage: "true" + ipForwardedHeader: X-Forwarded-For + logLevel: DEBUG + protectParameters: "false" + goodBots: [] + enableGooglebotIPCheck: "false" + enableUptimeRobotBypass: "false" + mode: regex + protectRoutes: + - "^/" + excludeRoutes: + - "\\/oai\\/request" + - "\\/node\\/\\d+\\/(book-)?manifest" diff --git a/ci/docker-compose.multilayer.yml b/ci/docker-compose.multilayer.yml new file mode 100644 index 0000000..0ef7fed --- /dev/null +++ b/ci/docker-compose.multilayer.yml @@ -0,0 +1,60 @@ +networks: + default: + +services: + nginx: + image: nginx:${NGINX_TAG} + healthcheck: + test: curl -fs http://localhost/healthz | grep -q OK || exit 1 + start_period: 5s + volumes: + - ./conf/nginx/default.conf:/etc/nginx/conf.d/default.conf:r + networks: + default: + aliases: + - nginx + + nginx2: + image: nginx:${NGINX_TAG} + healthcheck: + test: curl -fs http://localhost/healthz | grep -q OK || exit 1 + start_period: 5s + volumes: + - ./conf/nginx/default.conf:/etc/nginx/conf.d/default.conf:r + networks: + default: + aliases: + - nginx2 + + traefik: + image: traefik:${TRAEFIK_TAG} + command: >- + --api.insecure=true + --api.dashboard=true + --api.debug=true + --ping=true + --entryPoints.http.address=:80 + --entryPoints.http.forwardedHeaders.insecure=true + --entryPoints.http.forwardedHeaders.trustedIPs=127.0.0.1/32,::1/128,10.0.0.0/8,172.16.0.0/12,192.168.0.0/16 + --providers.file.filename=/etc/traefik/dynamic/multilayer.yml + --experimental.localPlugins.captcha-protect.moduleName=github.com/libops/captcha-protect + volumes: + - ./conf/traefik/multilayer.yml:/etc/traefik/dynamic/multilayer.yml:r + - ./../:/plugins-local/src/github.com/libops/captcha-protect:r + ports: + - "80:80" + - "8080:8080" + mem_limit: 256m + mem_reservation: 128m + networks: + default: + aliases: + - traefik + healthcheck: + test: traefik healthcheck --ping + start_period: 5s + depends_on: + nginx: + condition: service_healthy + nginx2: + condition: service_healthy diff --git a/ci/docker-compose.yml b/ci/docker-compose.yml index de8bfec..7e03626 100644 --- a/ci/docker-compose.yml +++ b/ci/docker-compose.yml @@ -12,7 +12,6 @@ services: traefik.http.routers.nginx.middlewares: captcha-protect@docker traefik.http.middlewares.captcha-protect.plugin.captcha-protect.captchaProvider: turnstile traefik.http.middlewares.captcha-protect.plugin.captcha-protect.window: 120 - traefik.http.middlewares.captcha-protect.plugin.captcha-protect.rateLimit: ${RATE_LIMIT} traefik.http.middlewares.captcha-protect.plugin.captcha-protect.siteKey: ${TURNSTILE_SITE_KEY} traefik.http.middlewares.captcha-protect.plugin.captcha-protect.secretKey: ${TURNSTILE_SECRET_KEY} traefik.http.middlewares.captcha-protect.plugin.captcha-protect.enableStatsPage: "true" @@ -25,8 +24,6 @@ services: traefik.http.middlewares.captcha-protect.plugin.captcha-protect.mode: "regex" traefik.http.middlewares.captcha-protect.plugin.captcha-protect.protectRoutes: "^/" traefik.http.middlewares.captcha-protect.plugin.captcha-protect.excludeRoutes: "\\/oai\\/request,\\/node\\/\\d+\\/(book-)?manifest" - traefik.http.middlewares.captcha-protect.plugin.captcha-protect.persistentStateFile: "/tmp/state.json" - traefik.http.middlewares.captcha-protect.plugin.captcha-protect.enableStateReconciliation: "true" healthcheck: test: curl -fs http://localhost/healthz | grep -q OK || exit 1 start_period: 5s @@ -47,7 +44,6 @@ services: traefik.http.routers.nginx2.middlewares: captcha-protect@docker traefik.http.middlewares.captcha-protect.plugin.captcha-protect.captchaProvider: turnstile traefik.http.middlewares.captcha-protect.plugin.captcha-protect.window: 120 - traefik.http.middlewares.captcha-protect.plugin.captcha-protect.rateLimit: ${RATE_LIMIT} traefik.http.middlewares.captcha-protect.plugin.captcha-protect.siteKey: ${TURNSTILE_SITE_KEY} traefik.http.middlewares.captcha-protect.plugin.captcha-protect.secretKey: ${TURNSTILE_SECRET_KEY} traefik.http.middlewares.captcha-protect.plugin.captcha-protect.enableStatsPage: "true" @@ -60,8 +56,6 @@ services: traefik.http.middlewares.captcha-protect.plugin.captcha-protect.mode: "regex" traefik.http.middlewares.captcha-protect.plugin.captcha-protect.protectRoutes: "^/" traefik.http.middlewares.captcha-protect.plugin.captcha-protect.excludeRoutes: "\\/oai\\/request,\\/node\\/\\d+\\/(book-)?manifest" - traefik.http.middlewares.captcha-protect.plugin.captcha-protect.persistentStateFile: "/tmp/state.json" - traefik.http.middlewares.captcha-protect.plugin.captcha-protect.enableStateReconciliation: "true" healthcheck: test: curl -fs http://localhost/healthz | grep -q OK || exit 1 start_period: 5s @@ -86,7 +80,6 @@ services: --experimental.localPlugins.captcha-protect.moduleName=github.com/libops/captcha-protect volumes: - /var/run/docker.sock:/var/run/docker.sock:z - - ./tmp:/tmp:rw - ./../:/plugins-local/src/github.com/libops/captcha-protect:r ports: - "80:80" diff --git a/ci/parse-stress-results/main.go b/ci/parse-stress-results/main.go deleted file mode 100644 index 9e8e09c..0000000 --- a/ci/parse-stress-results/main.go +++ /dev/null @@ -1,185 +0,0 @@ -package main - -import ( - "bufio" - "encoding/json" - "fmt" - "os" - "regexp" - "strconv" - "strings" -) - -type TestEvent struct { - Time string `json:"Time"` - Action string `json:"Action"` - Package string `json:"Package"` - Test string `json:"Test"` - Output string `json:"Output"` - Elapsed float64 `json:"Elapsed"` -} - -type TestResult struct { - Name string - Entries string - Size string - Time int64 // milliseconds - Threshold int64 // milliseconds - Passed bool -} - -func main() { - scanner := bufio.NewScanner(os.Stdin) - - // Patterns to extract data - sizePattern := regexp.MustCompile(`size: ([\d.]+) (KB|MB)`) - timePattern := regexp.MustCompile(`took (\d+)ms`) - - results := make(map[string]*TestResult) - currentTest := "" - - // Initialize known tests - results["Small"] = &TestResult{Name: "Small", Entries: "16 rate / 256 verified", Threshold: 500} - results["Medium"] = &TestResult{Name: "Medium", Entries: "256 rate / 65K verified", Threshold: 1000} - results["Large"] = &TestResult{Name: "Large", Entries: "1K rate / 262K verified", Threshold: 3000} - results["XLarge"] = &TestResult{Name: "XLarge", Entries: "4K rate / 1M verified", Threshold: 10000} - - for scanner.Scan() { - line := scanner.Text() - - var event TestEvent - if err := json.Unmarshal([]byte(line), &event); err != nil { - continue - } - - // Track which test we're in - if event.Action == "run" && strings.Contains(event.Test, "TestStateOperationsWithinThreshold/") { - parts := strings.Split(event.Test, "/") - if len(parts) >= 2 { - testLevel := parts[1] - if _, ok := results[testLevel]; ok { - currentTest = testLevel - } - } - } - - // Extract size from Marshal test - if event.Output != "" && strings.Contains(event.Output, "Marshal took") && strings.Contains(event.Output, "size:") { - if matches := sizePattern.FindStringSubmatch(event.Output); len(matches) > 2 { - if currentTest != "" && results[currentTest] != nil { - results[currentTest].Size = matches[1] + " " + matches[2] - } - } - } - - // Extract time from SaveStateToFile test - if event.Output != "" && strings.Contains(event.Output, "SaveStateToFile took") { - if matches := timePattern.FindStringSubmatch(event.Output); len(matches) > 1 { - if timeMs, err := strconv.ParseInt(matches[1], 10, 64); err == nil { - if currentTest != "" && results[currentTest] != nil { - results[currentTest].Time = timeMs - results[currentTest].Passed = timeMs <= results[currentTest].Threshold - } - } - } - } - } - - // Check if we should output Markdown (for CI) or plain text (for local) - inCI := os.Getenv("CI") != "" || os.Getenv("GITHUB_ACTIONS") != "" - - allPassed := true - order := []string{"Small", "Medium", "Large", "XLarge"} - - if inCI { - // Markdown table for GitHub PR comments - fmt.Println("\n### Stress Test Summary") - fmt.Println("| Scale | Entries | JSON Size | Time | Threshold | Status |") - fmt.Println("|-------|---------|-----------|------|-----------|--------|") - - for _, name := range order { - result := results[name] - if result.Time == 0 { - continue // Skip if no data - } - - size := result.Size - if size == "" { - size = "N/A" - } - - status := "✅" - if !result.Passed { - status = "❌" - allPassed = false - } - - thresholdStr := formatThreshold(result.Threshold) - - fmt.Printf("| %s | %s | %s | %dms | %s | %s |\n", - result.Name, - result.Entries, - size, - result.Time, - thresholdStr, - status) - } - fmt.Println() - - // In CI, just report metrics without failing - fmt.Println("ℹ️ Performance metrics reported (thresholds informational only in CI)") - if !allPassed { - fmt.Println(" Note: Some tests exceeded local development thresholds, but this is expected on CI runners") - } - - } else { - // ASCII table for local terminal - fmt.Println("\nStress Test Summary:") - fmt.Println("============================================================================================================") - fmt.Printf("%-8s | %-35s | %-10s | %-15s | %-10s | %-6s\n", "Scale", "Entries", "JSON Size", "Time", "Threshold", "Status") - fmt.Println("------------------------------------------------------------------------------------------------------------") - - for _, name := range order { - result := results[name] - if result.Time == 0 { - continue // Skip if no data - } - - size := result.Size - if size == "" { - size = "N/A" - } - - status := "✅" - if !result.Passed { - status = "❌" - allPassed = false - } - - thresholdStr := formatThreshold(result.Threshold) - - fmt.Printf("%-8s | %-35s | %-10s | %-15s | %-10s | %s\n", - result.Name, - result.Entries, - size, - fmt.Sprintf("%dms", result.Time), - thresholdStr, - status) - } - fmt.Println() - // Local development: enforce thresholds - if allPassed { - fmt.Println("✅ All stress tests passed within thresholds") - } else { - fmt.Println("❌ Some stress tests exceeded thresholds") - os.Exit(1) - } - } -} - -func formatThreshold(ms int64) string { - if ms < 1000 { - return fmt.Sprintf("<%dms", ms) - } - return fmt.Sprintf("<%ds", ms/1000) -} diff --git a/ci/run-stress-tests.sh b/ci/run-stress-tests.sh deleted file mode 100755 index fc16159..0000000 --- a/ci/run-stress-tests.sh +++ /dev/null @@ -1,32 +0,0 @@ -#!/usr/bin/env bash - -set -eou pipefail - -# Determine correct path to internal/state (works from root or ci/ directory) -if [ -d "./internal/state" ]; then - STATE_PATH="./internal/state" -elif [ -d "../internal/state" ]; then - STATE_PATH="../internal/state" -else - echo "Error: Cannot find internal/state directory" >&2 - exit 1 -fi - -# Determine correct path to parser (works from root or ci/ directory) -if [ -d "./ci/parse-stress-results" ]; then - PARSER_PATH="./ci/parse-stress-results" -elif [ -d "./parse-stress-results" ]; then - PARSER_PATH="./parse-stress-results" -else - echo "Error: Cannot find parse-stress-results directory" >&2 - exit 1 -fi - -echo "Running State Persistence Stress Tests" -echo "========================================" -echo "" -echo "Testing Small, Medium, Large, and XLarge scales..." -echo "" - -go test -json -run 'TestStateOperationsWithinThreshold' "$STATE_PATH" | \ - go run "$PARSER_PATH/main.go" diff --git a/ci/test.go b/ci/test.go index f4d2abb..b83d982 100755 --- a/ci/test.go +++ b/ci/test.go @@ -10,7 +10,6 @@ import ( "time" ) -const rateLimit = 5 const rootSmokeIP = "192.0.2.10" const app2SmokeIP = "198.51.100.10" @@ -55,10 +54,6 @@ func waitForService(url string) { } func assertProtectedRoute(ip, url, expectedURL string) { - for i := 0; i < rateLimit; i++ { - assertNoRedirect(ip, url) - } - output, err := httpRequest(ip, url) if err != nil { slog.Error("Request failed", "ip", ip, "url", url, "err", err) @@ -167,9 +162,9 @@ func runCommand(name string, args ...string) { } func testCommandEnv() []string { - env := append(os.Environ(), fmt.Sprintf("RATE_LIMIT=%d", rateLimit)) + env := os.Environ() if traefikTag := os.Getenv("TRAEFIK_TAG"); traefikTag != "" { - env = append(env, fmt.Sprintf("TRAEFIK_TAG=%s", traefikTag)) + env = append(env, "TRAEFIK_TAG="+traefikTag) } return env } diff --git a/ci_behavior_test.go b/ci_behavior_test.go index edfe291..ebff4fb 100644 --- a/ci_behavior_test.go +++ b/ci_behavior_test.go @@ -1,30 +1,23 @@ package captcha_protect import ( - "context" "io" "log/slog" "net/http" "net/http/httptest" - "path/filepath" "testing" - "testing/synctest" "time" "github.com/libops/captcha-protect/internal/helper" lru "github.com/patrickmn/go-cache" ) -const ciRateLimit = uint(5) const ciRootSmokeIP = "192.0.2.10" const ciApp2SmokeIP = "198.51.100.10" func TestCILabelEquivalentMiddlewareBehavior(t *testing.T) { bc := newCILabelEquivalentMiddleware(t, nil) - for i := uint(0); i < ciRateLimit; i++ { - assertNoRedirect(t, bc, ciRootSmokeIP, "/") - } assertRedirect(t, bc, ciRootSmokeIP, "/", "/challenge?destination=%2F") for _, route := range []string{ @@ -35,9 +28,6 @@ func TestCILabelEquivalentMiddlewareBehavior(t *testing.T) { assertNoRedirect(t, bc, ciRootSmokeIP, route) } - for i := uint(0); i < ciRateLimit; i++ { - assertNoRedirect(t, bc, ciApp2SmokeIP, "/app2") - } assertRedirect(t, bc, ciApp2SmokeIP, "/app2", "/challenge?destination=%2Fapp2") } @@ -49,9 +39,7 @@ func TestCILabelEquivalentGooglebotParameterBehavior(t *testing.T) { bypass.googlebotIPs.Update([]string{"203.0.113.0/24"}, discardLogger()) bypass.config.EnableGooglebotIPCheck = "true" - for i := uint(0); i < ciRateLimit+1; i++ { - assertNoRedirect(t, bypass, googleIP, "/") - } + assertNoRedirect(t, bypass, googleIP, "/") protectedParams := newCILabelEquivalentMiddleware(t, func(config *Config) { config.ProtectParameters = "true" @@ -60,9 +48,6 @@ func TestCILabelEquivalentGooglebotParameterBehavior(t *testing.T) { protectedParams.googlebotIPs.Update([]string{"203.0.113.0/24"}, discardLogger()) protectedParams.config.EnableGooglebotIPCheck = "true" - for i := uint(0); i < ciRateLimit; i++ { - assertNoRedirect(t, protectedParams, googleIP, "/?foo=bar") - } assertRedirect(t, protectedParams, googleIP, "/?foo=bar", "/challenge?destination=%2F%3Ffoo%3Dbar") } @@ -74,54 +59,14 @@ func TestCILabelEquivalentUptimeRobotBypassBehavior(t *testing.T) { bypass.uptimeRobotIPs.Update([]string{"203.0.113.10/32"}, discardLogger()) bypass.config.EnableUptimeRobotBypass = "true" - for i := uint(0); i < ciRateLimit+1; i++ { - assertNoRedirect(t, bypass, uptimeRobotIP, "/") - } + assertNoRedirect(t, bypass, uptimeRobotIP, "/") disabled := newCILabelEquivalentMiddleware(t, nil) disabled.uptimeRobotIPs = helper.NewUptimeRobotIPs() disabled.uptimeRobotIPs.Update([]string{"203.0.113.10/32"}, discardLogger()) - for i := uint(0); i < ciRateLimit; i++ { - assertNoRedirect(t, disabled, uptimeRobotIP, "/") - } assertRedirect(t, disabled, uptimeRobotIP, "/", "/challenge?destination=%2F") } -func TestPersistentStateSharingWithSynctest(t *testing.T) { - synctest.Test(t, func(t *testing.T) { - stateFile := filepath.Join(t.TempDir(), "state.json") - writer := newStateOnlyCaptchaProtect(stateFile, 2) - reader := newStateOnlyCaptchaProtect(stateFile, 2) - - ctx, cancel := context.WithCancel(t.Context()) - done := make(chan struct{}, 1) - go func() { - writer.saveState(ctx) - done <- struct{}{} - }() - - for i := uint(0); i < writer.config.RateLimit+1; i++ { - writer.registerRequest("192.0.0.0") - } - - time.Sleep(stateSaveInterval(writer.config) + StateSaveJitter + 3*time.Second) - synctest.Wait() - reader.reconcileStateFromFileIfChanged() - - v, ok := reader.rateCache.Get("192.0.0.0") - if !ok { - t.Fatal("expected reader instance to reconcile writer state") - } - if got, want := v.(uint), writer.config.RateLimit+1; got != want { - t.Fatalf("reconciled rate = %d, want %d", got, want) - } - - cancel() - synctest.Wait() - <-done - }) -} - func newCILabelEquivalentMiddleware(t *testing.T, mutate func(*Config)) *CaptchaProtect { t.Helper() @@ -143,7 +88,6 @@ func newCILabelEquivalentMiddleware(t *testing.T, mutate func(*Config)) *Captcha func ciLabelEquivalentConfig() *Config { config := CreateConfig() - config.RateLimit = ciRateLimit config.Window = 120 config.CaptchaProvider = "poj" config.SiteKey = "test-site-key" @@ -164,16 +108,13 @@ func ciLabelEquivalentConfig() *Config { return config } -func newStateOnlyCaptchaProtect(stateFile string, rateLimit uint) *CaptchaProtect { +func newStateOnlyCaptchaProtect(stateFile string) *CaptchaProtect { config := ciLabelEquivalentConfig() config.PersistentStateFile = stateFile - config.EnableStateReconciliation = "true" - config.RateLimit = rateLimit return &CaptchaProtect{ config: config, log: discardLogger(), - rateCache: lru.New(time.Hour, lru.NoExpiration), botCache: lru.New(time.Hour, lru.NoExpiration), verifiedCache: lru.New(time.Hour, lru.NoExpiration), } diff --git a/internal/state/state.go b/internal/state/state.go index d395778..2bbe80c 100644 --- a/internal/state/state.go +++ b/internal/state/state.go @@ -23,53 +23,37 @@ type CacheEntry struct { // State contains the persisted cache snapshot and approximate cache memory usage. type State struct { - Rate map[string]CacheEntry `json:"rate"` Verified map[string]CacheEntry `json:"verified"` Memory map[string]uintptr `json:"memory"` } // Keep concrete entry types: Traefik's Yaegi interpreter cannot reliably handle // generic persistentEntry[T] map fields. -type persistentRateEntry struct { - Value uint `json:"value"` - Expiration int64 `json:"expiration"` -} - type persistentBoolEntry struct { Value bool `json:"value"` Expiration int64 `json:"expiration"` } type persistentState struct { - Rate map[string]persistentRateEntry `json:"rate"` Verified map[string]persistentBoolEntry `json:"verified"` Memory map[string]uintptr `json:"memory"` } -type reconcileStateFile struct { - Rate map[string]persistentRateEntry `json:"rate"` - Verified map[string]persistentBoolEntry `json:"verified"` -} - // SaveMetrics reports timing and entry counts for a state save. type SaveMetrics struct { LockMs int64 - ReadMs int64 - ReconcileMs int64 MarshalMs int64 WriteMs int64 TotalMs int64 - RateEntries int VerifiedEntries int } // GetState converts cache items into a serializable state snapshot. -func GetState(rateCache, verifiedCache map[string]lru.Item) State { +func GetState(verifiedCache map[string]lru.Item) State { state := State{ - Memory: make(map[string]uintptr, 2), + Memory: make(map[string]uintptr, 1), } - state.Rate, state.Memory["rate"] = getCacheEntries[uint](rateCache) state.Verified, state.Memory["verified"] = getCacheEntries[bool](verifiedCache) return state @@ -77,28 +61,14 @@ func GetState(rateCache, verifiedCache map[string]lru.Item) State { // SetState loads state data into the provided caches, preserving expiration times. // If an entry has already expired (expiration < now), it will be skipped. -func SetState(state State, rateCache, verifiedCache *lru.Cache) { - loadCacheEntries(state.Rate, rateCache, convertRateValue) +func SetState(state State, verifiedCache *lru.Cache) { loadCacheEntries(state.Verified, verifiedCache, convertBoolValue) } -// ReconcileState merges file-based state with in-memory state. -func ReconcileState(fileState State, rateCache, verifiedCache *lru.Cache) { - rateItems := rateCache.Items() - verifiedItems := verifiedCache.Items() - - // Use "max value wins" for rate cache - reconcileRateCache(fileState.Rate, rateItems, rateCache, convertRateValue) - - // Use "later expiration wins" for verified caches - reconcileCacheEntries(fileState.Verified, verifiedItems, verifiedCache, convertBoolValue) -} - -// SaveStateToFileWithMetrics saves rate and verified state to a file with locking. +// SaveStateToFileWithMetrics saves verified state to a file with locking. func SaveStateToFileWithMetrics( filePath string, - reconcile bool, - rateCache, verifiedCache *lru.Cache, + verifiedCache *lru.Cache, log *slog.Logger, ) (SaveMetrics, error) { startTime := time.Now() @@ -114,30 +84,12 @@ func SaveStateToFileWithMetrics( return metrics, fmt.Errorf("failed to acquire lock: %w", err) } metrics.LockMs = time.Since(startTime).Milliseconds() + log.Debug("Saving state snapshot") - // Reconcile with existing file state if enabled - if reconcile { - readStart := time.Now() - fileContent, readErr := os.ReadFile(filePath) // #nosec G304 -- persistent state path is trusted middleware configuration. - metrics.ReadMs = time.Since(readStart).Milliseconds() - - if readErr == nil && len(fileContent) > 0 { - reconcileStart := time.Now() - var fileState reconcileStateFile - if unmarshalErr := json.Unmarshal(fileContent, &fileState); unmarshalErr == nil { - log.Debug("Reconciling state before save", "fileBytes", len(fileContent)) - reconcilePersistentFileState(fileState, rateCache, verifiedCache) - } - metrics.ReconcileMs = time.Since(reconcileStart).Milliseconds() - } - } - - rateItems := rateCache.Items() verifiedItems := verifiedCache.Items() - metrics.RateEntries = len(rateItems) metrics.VerifiedEntries = len(verifiedItems) - metrics.MarshalMs, metrics.WriteMs, err = atomicWriteStateFile(filePath, rateItems, verifiedItems, 0600) + metrics.MarshalMs, metrics.WriteMs, err = atomicWriteStateFile(filePath, verifiedItems, 0600) if err != nil { return metrics, err } @@ -148,7 +100,7 @@ func SaveStateToFileWithMetrics( func atomicWriteStateFile( filePath string, - rateItems, verifiedItems map[string]lru.Item, + verifiedItems map[string]lru.Item, perm os.FileMode, ) (marshalMs, writeMs int64, err error) { dir := filepath.Dir(filePath) @@ -161,7 +113,7 @@ func atomicWriteStateFile( writer := bufio.NewWriterSize(tmp, 1024*1024) marshalStart := time.Now() - if err := writeStateJSON(writer, rateItems, verifiedItems); err != nil { + if err := writeStateJSON(writer, verifiedItems); err != nil { _ = tmp.Close() return 0, 0, err } @@ -188,16 +140,9 @@ func atomicWriteStateFile( func writeStateJSON( writer *bufio.Writer, - rateItems, verifiedItems map[string]lru.Item, + verifiedItems map[string]lru.Item, ) error { - if err := writeString(writer, `{"rate":`); err != nil { - return err - } - rateMemory, err := writeCacheEntryMap[uint](writer, rateItems, writeUint) - if err != nil { - return err - } - if err := writeString(writer, `,"verified":`); err != nil { + if err := writeString(writer, `{"verified":`); err != nil { return err } verifiedMemory, err := writeCacheEntryMap[bool](writer, verifiedItems, writeBool) @@ -205,13 +150,7 @@ func writeStateJSON( return err } - if err := writeString(writer, `,"memory":{"rate":`); err != nil { - return err - } - if err := writeString(writer, strconv.FormatUint(uint64(rateMemory), 10)); err != nil { - return err - } - if err := writeString(writer, `,"verified":`); err != nil { + if err := writeString(writer, `,"memory":{"verified":`); err != nil { return err } if err := writeString(writer, strconv.FormatUint(uint64(verifiedMemory), 10)); err != nil { @@ -280,10 +219,6 @@ var ( lruItemSize = reflect.TypeOf(lru.Item{}).Size() ) -func writeUint(writer *bufio.Writer, value uint) error { - return writeString(writer, strconv.FormatUint(uint64(value), 10)) -} - func writeBool(writer *bufio.Writer, value bool) error { if value { return writeString(writer, "true") @@ -336,7 +271,7 @@ func isPlainJSONString(value string) bool { // LoadStateFromFile loads state from a file with locking. func LoadStateFromFile( filePath string, - rateCache, verifiedCache *lru.Cache, + verifiedCache *lru.Cache, ) error { lock, err := NewFileLock(filePath + ".lock") if err != nil { @@ -359,37 +294,8 @@ func LoadStateFromFile( return err } - setPersistentState(loadedState, rateCache, verifiedCache) - - return nil -} - -// ReconcileStateFromFile merges newer persisted state into the provided caches. -func ReconcileStateFromFile( - filePath string, - rateCache, verifiedCache *lru.Cache, -) error { - lock, err := NewFileLock(filePath + ".lock") - if err != nil { - return err - } - defer lock.Close() - - if err := lock.Lock(); err != nil { - return err - } - - fileContent, err := os.ReadFile(filePath) // #nosec G304 -- persistent state path is trusted middleware configuration. - if err != nil || len(fileContent) == 0 { - return err - } - - var fileState reconcileStateFile - if err := json.Unmarshal(fileContent, &fileState); err != nil { - return err - } + setPersistentState(loadedState, verifiedCache) - reconcilePersistentFileState(fileState, rateCache, verifiedCache) return nil } @@ -400,19 +306,6 @@ func calculateDuration(expiration int64, now int64) time.Duration { return time.Duration(expiration - now) } -func convertRateValue(v interface{}) (uint, bool) { - switch val := v.(type) { - case uint: - return val, true - case float64: - return uint(val), true - case int: - return uint(val), true - default: - return 0, false - } -} - func convertBoolValue(v interface{}) (bool, bool) { switch val := v.(type) { case bool: @@ -458,21 +351,10 @@ func loadCacheEntries[T any]( } } -func setPersistentState(state persistentState, rateCache, verifiedCache *lru.Cache) { - loadPersistentRateEntries(state.Rate, rateCache) +func setPersistentState(state persistentState, verifiedCache *lru.Cache) { loadPersistentBoolEntries(state.Verified, verifiedCache) } -func loadPersistentRateEntries(entries map[string]persistentRateEntry, cache *lru.Cache) { - now := time.Now().UnixNano() - for key, entry := range entries { - if entry.Expiration > 0 && entry.Expiration <= now { - continue - } - cache.Set(key, entry.Value, calculateDuration(entry.Expiration, now)) - } -} - func loadPersistentBoolEntries(entries map[string]persistentBoolEntry, cache *lru.Cache) { now := time.Now().UnixNano() for key, entry := range entries { @@ -482,158 +364,3 @@ func loadPersistentBoolEntries(entries map[string]persistentBoolEntry, cache *lr cache.Set(key, entry.Value, calculateDuration(entry.Expiration, now)) } } - -func reconcilePersistentFileState(state reconcileStateFile, rateCache, verifiedCache *lru.Cache) { - rateItems := rateCache.Items() - verifiedItems := verifiedCache.Items() - - reconcilePersistentRateCache(state.Rate, rateItems, rateCache) - reconcilePersistentBoolCacheEntries(state.Verified, verifiedItems, verifiedCache) -} - -func reconcilePersistentBoolCacheEntries( - fileEntries map[string]persistentBoolEntry, - memItems map[string]lru.Item, - cache *lru.Cache, -) { - now := time.Now().UnixNano() - for key, fileEntry := range fileEntries { - if fileEntry.Expiration > 0 && fileEntry.Expiration <= now { - continue - } - - duration := calculateDuration(fileEntry.Expiration, now) - memItem, exists := memItems[key] - if !exists { - cache.Set(key, fileEntry.Value, duration) - continue - } - - if fileEntry.Expiration > memItem.Expiration { - cache.Set(key, fileEntry.Value, duration) - } - } -} - -func reconcilePersistentRateCache( - fileEntries map[string]persistentRateEntry, - memItems map[string]lru.Item, - cache *lru.Cache, -) { - now := time.Now().UnixNano() - for key, fileEntry := range fileEntries { - if fileEntry.Expiration > 0 && fileEntry.Expiration <= now { - continue - } - - memItem, exists := memItems[key] - if !exists { - cache.Set(key, fileEntry.Value, calculateDuration(fileEntry.Expiration, now)) - continue - } - - memValue, ok := memItem.Object.(uint) - if !ok { - cache.Set(key, fileEntry.Value, calculateDuration(fileEntry.Expiration, now)) - continue - } - - combinedValue := maxUint(fileEntry.Value, memValue) - laterExpiration := max(fileEntry.Expiration, memItem.Expiration) - cache.Set(key, combinedValue, calculateDuration(laterExpiration, now)) - } -} - -// reconcileCacheEntries implements "later expiration wins" for bool flags. -func reconcileCacheEntries[T any]( - fileEntries map[string]CacheEntry, - memItems map[string]lru.Item, - cache *lru.Cache, - converter func(interface{}) (T, bool), -) { - now := time.Now().UnixNano() - for k, fileEntry := range fileEntries { - if fileEntry.Expiration > 0 && fileEntry.Expiration <= now { - continue - } - - value, ok := converter(fileEntry.Value) - if !ok { - continue - } - - duration := calculateDuration(fileEntry.Expiration, now) - - memItem, exists := memItems[k] - if !exists { - cache.Set(k, value, duration) - continue - } - - if fileEntry.Expiration > memItem.Expiration { - cache.Set(k, value, duration) - } - } -} - -// reconcileRateCache implements "max value wins" and "max expiration wins". -// This prevents runaway growth (from summing) and accepts data loss -// (under-counting) as the safer alternative. -func reconcileRateCache( - fileEntries map[string]CacheEntry, - memItems map[string]lru.Item, - cache *lru.Cache, - converter func(interface{}) (uint, bool), -) { - now := time.Now().UnixNano() - for k, fileEntry := range fileEntries { - if fileEntry.Expiration > 0 && fileEntry.Expiration <= now { - continue - } - - fileValue, ok := converter(fileEntry.Value) - if !ok { - continue - } - - memItem, exists := memItems[k] - if !exists { - // Entry only in file, just add it - duration := calculateDuration(fileEntry.Expiration, now) - cache.Set(k, fileValue, duration) - continue - } - - // Entry in both, combine them - memValue, ok := memItem.Object.(uint) - if !ok { - // In-memory object is not uint, something is wrong. - // Overwrite with file value as a fallback. - duration := calculateDuration(fileEntry.Expiration, now) - cache.Set(k, fileValue, duration) - continue - } - - // Use the HIGHEST value, not the sum - combinedValue := maxUint(fileValue, memValue) - // Use the LATER expiration - laterExpiration := max(fileEntry.Expiration, memItem.Expiration) - - duration := calculateDuration(laterExpiration, now) - cache.Set(k, combinedValue, duration) - } -} - -func max(a, b int64) int64 { - if a > b { - return a - } - return b -} - -func maxUint(a, b uint) uint { - if a > b { - return a - } - return b -} diff --git a/internal/state/state_stress_test.go b/internal/state/state_stress_test.go deleted file mode 100644 index 732d2c5..0000000 --- a/internal/state/state_stress_test.go +++ /dev/null @@ -1,395 +0,0 @@ -package state - -import ( - "bufio" - "bytes" - "encoding/json" - "fmt" - "log/slog" - "math" - "testing" - "time" - - lru "github.com/patrickmn/go-cache" -) - -// This file contains stress tests for state persistence operations at various scales. -// -// Performance Findings (Apple M2 Pro): -// Small (16 rate / 256 verified): -// - SaveStateToFile with reconciliation: ~84ms -// Medium (256 rate / 65K verified): -// - SaveStateToFile with reconciliation: ~410ms -// Large (1,024 rate / 262K verified): -// - SaveStateToFile with reconciliation: ~1.8s -// XLarge (4,096 rate / 1M verified): -// - SaveStateToFile with reconciliation: ~8.7s (approaching 10s save window limit) -// -// Recommendation: Do not enable enableStateReconciliation for sites with >1M unique visitors. -// The reconciliation overhead at scale (5-8s) conflicts with the 10-second save interval. -// -// NOTE: The ci/run-stress-tests.sh script reports these metrics. In CI environments, -// thresholds are informational only (won't fail the build) since CI runners vary in -// performance. Thresholds are enforced in local development. - -// StressLevel defines the size parameters for a stress test -type StressLevel struct { - Name string - RateEntries int - VerifiedEntries int -} - -// getStressLevels returns the configured stress test levels -// Note: We cap large levels at practical sizes to avoid exhausting system memory -func getStressLevels() []StressLevel { - return []StressLevel{ - { - Name: "Small", - RateEntries: 1 << 4, // 2^4 = 16 - VerifiedEntries: 1 << 8, // 2^8 = 256 - }, - { - Name: "Medium", - RateEntries: 1 << 8, // 2^8 = 256 - VerifiedEntries: 1 << 16, // 2^16 = 65,536 - }, - { - Name: "Large", - RateEntries: 1 << 10, // 2^10 = 1,024 (capped from 2^16) - VerifiedEntries: 1 << 18, // 2^18 = 262,144 (capped from 2^32) - }, - { - Name: "XLarge", - RateEntries: 1 << 12, // 2^12 = 4,096 - VerifiedEntries: 1 << 20, // 2^20 = 1,048,576 - }, - } -} - -// generateIPv4Subnet generates a unique IPv4 subnet string for the rate cache -// Uses the pattern: A.B.0.0 where A and B are derived from the index -func generateIPv4Subnet(index int) string { - // Create /16 subnets (e.g., 10.0.0.0, 10.1.0.0, etc.) - a := (index >> 8) & 0xFF - b := index & 0xFF - return fmt.Sprintf("%d.%d.0.0", a, b) -} - -// generateIPv4Address generates a unique IPv4 address for verified caches -// Uses the pattern: A.B.C.D where all octets are derived from the index -func generateIPv4Address(index int) string { - a := (index >> 24) & 0xFF - b := (index >> 16) & 0xFF - c := (index >> 8) & 0xFF - d := index & 0xFF - return fmt.Sprintf("%d.%d.%d.%d", a, b, c, d) -} - -// populateCaches fills caches with test data based on the stress level -func populateCaches(level StressLevel, rateCache, verifiedCache *lru.Cache) { - expiration := 24 * time.Hour - - // Populate rate cache with subnet entries - for i := 0; i < level.RateEntries; i++ { - subnet := generateIPv4Subnet(i) - // Vary the rate values (1-100) - rate := uint(1 + (i % 100)) - rateCache.Set(subnet, rate, expiration) - } - - // Populate verified cache with IP addresses - startOffset := 0x10000000 // Start from 16.0.0.0 - for i := 0; i < level.VerifiedEntries; i++ { - ip := generateIPv4Address(startOffset + i) - verifiedCache.Set(ip, true, expiration) - } -} - -// BenchmarkStateOperations benchmarks marshal/unmarshal/reconcile at different scales -func BenchmarkStateOperations(b *testing.B) { - levels := getStressLevels() - - for _, level := range levels { - // Create caches and populate with test data - rateCache := lru.New(24*time.Hour, lru.NoExpiration) - verifiedCache := lru.New(24*time.Hour, lru.NoExpiration) - - b.Logf("Populating caches for %s level (rate=%d, verified=%d)...", - level.Name, level.RateEntries, level.VerifiedEntries) - populateCaches(level, rateCache, verifiedCache) - - // Benchmark GetState (extract to struct) - b.Run(fmt.Sprintf("GetState/%s", level.Name), func(b *testing.B) { - b.ReportAllocs() - b.ResetTimer() - for i := 0; i < b.N; i++ { - _ = GetState(rateCache.Items(), verifiedCache.Items()) - } - }) - - // Benchmark Marshal - state := GetState(rateCache.Items(), verifiedCache.Items()) - b.Run(fmt.Sprintf("Marshal/%s", level.Name), func(b *testing.B) { - b.ReportAllocs() - var jsonData []byte - b.ResetTimer() - for i := 0; i < b.N; i++ { - jsonData, _ = json.Marshal(state) - } - b.ReportMetric(float64(len(jsonData)), "bytes") - }) - - // Benchmark Unmarshal - jsonData, _ := json.Marshal(state) - b.Run(fmt.Sprintf("Unmarshal/%s", level.Name), func(b *testing.B) { - b.ReportAllocs() - b.ResetTimer() - for i := 0; i < b.N; i++ { - var loadedState State - _ = json.Unmarshal(jsonData, &loadedState) - } - }) - - // Benchmark SetState (load into caches) - b.Run(fmt.Sprintf("SetState/%s", level.Name), func(b *testing.B) { - b.ReportAllocs() - b.ResetTimer() - for i := 0; i < b.N; i++ { - b.StopTimer() - newRateCache := lru.New(24*time.Hour, lru.NoExpiration) - newVerifiedCache := lru.New(24*time.Hour, lru.NoExpiration) - b.StartTimer() - - SetState(state, newRateCache, newVerifiedCache) - } - }) - - // Benchmark ReconcileState - b.Run(fmt.Sprintf("ReconcileState/%s", level.Name), func(b *testing.B) { - b.ReportAllocs() - b.ResetTimer() - for i := 0; i < b.N; i++ { - b.StopTimer() - // Create fresh caches with some overlapping data - newRateCache := lru.New(24*time.Hour, lru.NoExpiration) - newVerifiedCache := lru.New(24*time.Hour, lru.NoExpiration) - // Pre-populate with 50% of entries - for j := 0; j < level.RateEntries/2; j++ { - subnet := generateIPv4Subnet(j) - newRateCache.Set(subnet, uint(50), 24*time.Hour) - } - b.StartTimer() - - ReconcileState(state, newRateCache, newVerifiedCache) - } - }) - - // Benchmark full SaveStateToFileWithMetrics cycle (with reconciliation) - b.Run(fmt.Sprintf("SaveStateToFileWithMetrics/%s", level.Name), func(b *testing.B) { - tmpDir := b.TempDir() - tmpFile := tmpDir + "/state.json" - logger := testLogger() - - b.ReportAllocs() - b.ResetTimer() - for i := 0; i < b.N; i++ { - metrics, err := SaveStateToFileWithMetrics( - tmpFile, - true, // enable reconciliation - rateCache, - verifiedCache, - logger, - ) - if err != nil { - b.Fatalf("SaveStateToFile failed: %v", err) - } - - // Report timing breakdown (only once to avoid noise) - if i == 0 { - b.Logf("Timing breakdown: lock=%dms read=%dms reconcile=%dms marshal=%dms write=%dms total=%dms", - metrics.LockMs, metrics.ReadMs, metrics.ReconcileMs, metrics.MarshalMs, metrics.WriteMs, metrics.TotalMs) - } - } - }) - } -} - -// TestStateOperationsWithinThreshold ensures operations complete within acceptable time limits -func TestStateOperationsWithinThreshold(t *testing.T) { - if testing.Short() { - t.Skip("Skipping stress test in short mode") - } - - levels := getStressLevels() - - // Define thresholds for persistence operations (in milliseconds) - // These are generous limits to avoid flaky tests on slower CI machines - type thresholds struct { - MarshalMs int64 - - SaveWithReconcileMs int64 - } - - levelThresholds := map[string]thresholds{ - "Small": { - MarshalMs: 200, - - SaveWithReconcileMs: 500, - }, - "Medium": { - MarshalMs: 500, - - SaveWithReconcileMs: 1000, - }, - "Large": { - MarshalMs: 2000, - - SaveWithReconcileMs: 3000, - }, - "XLarge": { - MarshalMs: 5000, - - SaveWithReconcileMs: 10000, - }, - "XXLarge": { - MarshalMs: 15000, - - SaveWithReconcileMs: 30000, - }, - } - - for _, level := range levels { - t.Run(level.Name, func(t *testing.T) { - // Create and populate caches - rateCache := lru.New(24*time.Hour, lru.NoExpiration) - verifiedCache := lru.New(24*time.Hour, lru.NoExpiration) - - t.Logf("Populating persistent caches (rate=%d, verified=%d)...", - level.RateEntries, level.VerifiedEntries) - populateCaches(level, rateCache, verifiedCache) - - thresh := levelThresholds[level.Name] - - // Test Marshal - t.Run("Marshal", func(t *testing.T) { - start := time.Now() - jsonData, err := marshalPersistentSnapshotForStress(rateCache, verifiedCache) - elapsed := time.Since(start).Milliseconds() - - if err != nil { - t.Fatalf("Marshal failed: %v", err) - } - - sizeKB := float64(len(jsonData)) / 1024.0 - sizeMB := sizeKB / 1024.0 - if sizeMB >= 1.0 { - t.Logf("Marshal took %dms (threshold: %dms), size: %.2f MB", - elapsed, thresh.MarshalMs, sizeMB) - } else { - t.Logf("Marshal took %dms (threshold: %dms), size: %.2f KB", - elapsed, thresh.MarshalMs, sizeKB) - } - - if elapsed > thresh.MarshalMs { - slog.Error(fmt.Sprintf("Marshal took %dms, exceeds threshold of %dms", elapsed, thresh.MarshalMs)) - } - }) - - // Test full SaveStateToFileWithMetrics with reconciliation - t.Run("SaveStateToFileWithMetrics", func(t *testing.T) { - tmpFile := t.TempDir() + "/state.json" - logger := testLogger() - - // Pre-create a state file to enable reconciliation - if _, err := SaveStateToFileWithMetrics( - tmpFile, - false, - rateCache, - verifiedCache, - logger, - ); err != nil { - t.Fatalf("Failed to write initial state: %v", err) - } - - start := time.Now() - metrics, err := SaveStateToFileWithMetrics( - tmpFile, - true, // enable reconciliation - rateCache, - verifiedCache, - logger, - ) - elapsed := time.Since(start).Milliseconds() - - if err != nil { - t.Fatalf("SaveStateToFile failed: %v", err) - } - - t.Logf("SaveStateToFile took %dms (threshold: %dms)", elapsed, thresh.SaveWithReconcileMs) - t.Logf(" Breakdown: lock=%dms read=%dms reconcile=%dms marshal=%dms write=%dms total=%dms", - metrics.LockMs, metrics.ReadMs, metrics.ReconcileMs, metrics.MarshalMs, metrics.WriteMs, metrics.TotalMs) - - if elapsed > thresh.SaveWithReconcileMs { - slog.Error(fmt.Sprintf("SaveStateToFile took %dms, exceeds threshold of %dms", - elapsed, thresh.SaveWithReconcileMs)) - } - - // Verify math adds up (approximately, allowing for measurement overhead) - measuredTotal := metrics.LockMs + metrics.ReadMs + metrics.ReconcileMs + metrics.MarshalMs + metrics.WriteMs - if metrics.TotalMs > 0 && math.Abs(float64(measuredTotal-metrics.TotalMs)) > float64(metrics.TotalMs)*0.2 { - t.Logf("Warning: timing components (%dms) don't add up to total (%dms)", - measuredTotal, metrics.TotalMs) - } - }) - }) - } -} - -// TestGenerateUniqueIPs verifies IP generation produces unique addresses -func TestGenerateUniqueIPs(t *testing.T) { - t.Run("Subnets are unique", func(t *testing.T) { - count := 1000 - seen := make(map[string]bool, count) - - for i := 0; i < count; i++ { - subnet := generateIPv4Subnet(i) - if seen[subnet] { - t.Errorf("Duplicate subnet generated: %s", subnet) - } - seen[subnet] = true - } - - if len(seen) != count { - t.Errorf("Expected %d unique subnets, got %d", count, len(seen)) - } - }) - - t.Run("Addresses are unique", func(t *testing.T) { - count := 10000 - seen := make(map[string]bool, count) - - for i := 0; i < count; i++ { - ip := generateIPv4Address(i) - if seen[ip] { - t.Errorf("Duplicate IP generated: %s", ip) - } - seen[ip] = true - } - - if len(seen) != count { - t.Errorf("Expected %d unique IPs, got %d", count, len(seen)) - } - }) -} - -func marshalPersistentSnapshotForStress(rateCache, verifiedCache *lru.Cache) ([]byte, error) { - var buf bytes.Buffer - writer := bufio.NewWriter(&buf) - if err := writeStateJSON(writer, rateCache.Items(), verifiedCache.Items()); err != nil { - return nil, err - } - if err := writer.Flush(); err != nil { - return nil, err - } - return buf.Bytes(), nil -} diff --git a/internal/state/state_test.go b/internal/state/state_test.go index e9e0a4b..4a234f8 100644 --- a/internal/state/state_test.go +++ b/internal/state/state_test.go @@ -15,52 +15,23 @@ import ( ) func TestGetState(t *testing.T) { - // Create test caches - rateCache := lru.New(1*time.Hour, 1*time.Minute) verifiedCache := lru.New(1*time.Hour, 1*time.Minute) - - // Add test data - rateCache.Set("192.168.0.0", uint(10), lru.DefaultExpiration) - rateCache.Set("10.0.0.0", uint(5), lru.DefaultExpiration) - verifiedCache.Set("9.9.9.9", true, lru.DefaultExpiration) - // Get state - state := GetState(rateCache.Items(), verifiedCache.Items()) - - // Verify rate cache data - if len(state.Rate) != 2 { - t.Errorf("Expected 2 rate entries, got %d", len(state.Rate)) - } - if state.Rate["192.168.0.0"].Value != uint(10) { - t.Errorf("Expected rate 10 for 192.168.0.0, got %v", state.Rate["192.168.0.0"].Value) - } - if state.Rate["10.0.0.0"].Value != uint(5) { - t.Errorf("Expected rate 5 for 10.0.0.0, got %v", state.Rate["10.0.0.0"].Value) - } - // Verify expiration timestamps are set - if state.Rate["192.168.0.0"].Expiration == 0 { - t.Error("Expected non-zero expiration for 192.168.0.0") - } + state := GetState(verifiedCache.Items()) - // Verify verified cache data if len(state.Verified) != 1 { t.Errorf("Expected 1 verified entry, got %d", len(state.Verified)) } if state.Verified["9.9.9.9"].Value != true { t.Error("Expected 9.9.9.9 to be verified") } - // Verify expiration timestamp is set if state.Verified["9.9.9.9"].Expiration == 0 { t.Error("Expected non-zero expiration for verified 9.9.9.9") } - // Verify memory tracking exists - if len(state.Memory) != 2 { - t.Errorf("Expected 2 memory entries, got %d", len(state.Memory)) - } - if state.Memory["rate"] == 0 { - t.Error("Expected non-zero memory for rate cache") + if len(state.Memory) != 1 { + t.Errorf("Expected 1 memory entry, got %d", len(state.Memory)) } if state.Memory["verified"] == 0 { t.Error("Expected non-zero memory for verified cache") @@ -68,58 +39,33 @@ func TestGetState(t *testing.T) { } func TestGetStateEmpty(t *testing.T) { - // Create empty caches - rateCache := lru.New(1*time.Hour, 1*time.Minute) verifiedCache := lru.New(1*time.Hour, 1*time.Minute) - state := GetState(rateCache.Items(), verifiedCache.Items()) + state := GetState(verifiedCache.Items()) - if len(state.Rate) != 0 { - t.Errorf("Expected 0 rate entries, got %d", len(state.Rate)) - } if len(state.Verified) != 0 { t.Errorf("Expected 0 verified entries, got %d", len(state.Verified)) } - if len(state.Memory) != 2 { - t.Errorf("Expected 2 memory entries, got %d", len(state.Memory)) + if len(state.Memory) != 1 { + t.Errorf("Expected 1 memory entry, got %d", len(state.Memory)) } } func TestSetState(t *testing.T) { - // Create state with expiration times now := time.Now().UnixNano() futureExpiration := now + int64(1*time.Hour) pastExpiration := now - int64(1*time.Hour) state := State{ - Rate: map[string]CacheEntry{ - "192.168.0.0": {Value: uint(10), Expiration: futureExpiration}, - "10.0.0.0": {Value: uint(5), Expiration: pastExpiration}, // expired - }, Verified: map[string]CacheEntry{ "9.9.9.9": {Value: true, Expiration: futureExpiration}, - "8.8.8.8": {Value: true, Expiration: pastExpiration}, // expired - "7.7.7.7": {Value: true, Expiration: 0}, // no expiration + "8.8.8.8": {Value: true, Expiration: pastExpiration}, + "7.7.7.7": {Value: true, Expiration: 0}, }, } - // Create caches - rateCache := lru.New(1*time.Hour, 1*time.Minute) verifiedCache := lru.New(1*time.Hour, 1*time.Minute) - - // Set state - SetState(state, rateCache, verifiedCache) - - // Verify only non-expired entries were loaded - if rateCache.ItemCount() != 1 { - t.Errorf("Expected 1 rate entry (expired filtered out), got %d", rateCache.ItemCount()) - } - if v, ok := rateCache.Get("192.168.0.0"); !ok || v.(uint) != 10 { - t.Error("Expected rate 10 for 192.168.0.0") - } - if _, ok := rateCache.Get("10.0.0.0"); ok { - t.Error("Expected expired entry 10.0.0.0 to be filtered out") - } + SetState(state, verifiedCache) if verifiedCache.ItemCount() != 2 { t.Errorf("Expected 2 verified entries (1 expired filtered out), got %d", verifiedCache.ItemCount()) @@ -135,110 +81,25 @@ func TestSetState(t *testing.T) { } } -func TestReconcileState(t *testing.T) { - now := time.Now().UnixNano() - oldExpiration := now + int64(30*time.Minute) - newExpiration := now + int64(1*time.Hour) - - // Create file state with some entries - fileState := State{ - Rate: map[string]CacheEntry{ - "192.168.0.0": {Value: uint(15), Expiration: newExpiration}, // newer than memory - "10.0.0.0": {Value: uint(3), Expiration: oldExpiration}, // older than memory - "172.16.0.0": {Value: uint(7), Expiration: newExpiration}, // only in file - }, - Verified: map[string]CacheEntry{ - "1.1.1.1": {Value: true, Expiration: newExpiration}, // only in file - "2.2.2.2": {Value: true, Expiration: oldExpiration}, // older than memory - }, - } - - // Create memory caches with some overlapping data - rateCache := lru.New(1*time.Hour, 1*time.Minute) - verifiedCache := lru.New(1*time.Hour, 1*time.Minute) - - rateCache.Set("192.168.0.0", uint(10), time.Duration(oldExpiration-now)) // older, should be replaced - rateCache.Set("10.0.0.0", uint(5), time.Duration(newExpiration-now)) // newer, should be kept - rateCache.Set("8.8.8.8", uint(20), time.Duration(newExpiration-now)) // only in memory - - verifiedCache.Set("2.2.2.2", true, time.Duration(newExpiration-now)) // newer, should be kept - - // Reconcile - ReconcileState(fileState, rateCache, verifiedCache) - - // Verify reconciliation results - // 192.168.0.0 should be updated to file's value (newer expiration) - if v, ok := rateCache.Get("192.168.0.0"); !ok || v.(uint) != 15 { - t.Errorf("Expected rate 15 for 192.168.0.0 after reconciliation, got %v", v) - } - - // 10.0.0.0 should keep memory value (newer expiration) - if v, ok := rateCache.Get("10.0.0.0"); !ok || v.(uint) != 5 { - t.Errorf("Expected rate 5 for 10.0.0.0 (memory kept), got %v", v) - } - - // 172.16.0.0 should be added from file - if v, ok := rateCache.Get("172.16.0.0"); !ok || v.(uint) != 7 { - t.Error("Expected 172.16.0.0 to be added from file") - } - - // 8.8.8.8 should still exist (only in memory) - if v, ok := rateCache.Get("8.8.8.8"); !ok || v.(uint) != 20 { - t.Error("Expected 8.8.8.8 to still exist in memory") - } - - // 1.1.1.1 should be added from file - if v, ok := verifiedCache.Get("1.1.1.1"); !ok || v.(bool) != true { - t.Error("Expected 1.1.1.1 to be added from file") - } - - // 2.2.2.2 should keep memory value (newer expiration) - if v, ok := verifiedCache.Get("2.2.2.2"); !ok || v.(bool) != true { - t.Error("Expected 2.2.2.2 to be kept from memory") - } -} - func TestSaveStateToFileWithMetrics(t *testing.T) { - t.Run("Basic save without reconciliation", func(t *testing.T) { - // Create temp file + t.Run("Basic save", func(t *testing.T) { tmpFile := t.TempDir() + "/state.json" - - // Create caches with test data - rateCache := lru.New(1*time.Hour, 1*time.Minute) verifiedCache := lru.New(1*time.Hour, 1*time.Minute) - - rateCache.Set("192.168.0.0", uint(10), lru.DefaultExpiration) verifiedCache.Set("5.6.7.8", true, lru.DefaultExpiration) - // Save without reconciliation metrics, err := SaveStateToFileWithMetrics( tmpFile, - false, // no reconciliation - rateCache, verifiedCache, testLogger(), ) - if err != nil { t.Fatalf("SaveStateToFile failed: %v", err) } - // Verify timing metrics - if metrics.LockMs < 0 || metrics.ReadMs < 0 || metrics.ReconcileMs < 0 || metrics.MarshalMs < 0 || metrics.WriteMs < 0 || metrics.TotalMs < 0 { + if metrics.LockMs < 0 || metrics.MarshalMs < 0 || metrics.WriteMs < 0 || metrics.TotalMs < 0 { t.Error("Expected all timing metrics to be non-negative") } - // Verify reconcileMs is 0 when reconciliation is disabled - if metrics.ReconcileMs != 0 { - t.Errorf("Expected reconcileMs to be 0 when reconciliation disabled, got %d", metrics.ReconcileMs) - } - - // Verify readMs is 0 when reconciliation is disabled - if metrics.ReadMs != 0 { - t.Errorf("Expected readMs to be 0 when reconciliation disabled, got %d", metrics.ReadMs) - } - - // Verify file was created and contains data fileInfo, err := os.Stat(tmpFile) if err != nil { t.Fatalf("Failed to stat file: %v", err) @@ -250,7 +111,6 @@ func TestSaveStateToFileWithMetrics(t *testing.T) { t.Fatalf("State file mode = %v, want 0600", mode) } - // Load and verify the saved data savedData, err := os.ReadFile(tmpFile) if err != nil { t.Fatalf("Failed to read saved file: %v", err) @@ -260,98 +120,18 @@ func TestSaveStateToFileWithMetrics(t *testing.T) { if err := json.Unmarshal(savedData, &savedState); err != nil { t.Fatalf("Failed to unmarshal saved state: %v", err) } - - if len(savedState.Rate) != 1 { - t.Errorf("Expected 1 rate entry, got %d", len(savedState.Rate)) - } if len(savedState.Verified) != 1 { t.Errorf("Expected 1 verified entry, got %d", len(savedState.Verified)) } }) - t.Run("Save with reconciliation", func(t *testing.T) { - tmpFile := t.TempDir() + "/state.json" - - // Create initial state file - now := time.Now().UnixNano() - futureExpiration := now + int64(1*time.Hour) - initialState := State{ - Rate: map[string]CacheEntry{ - "10.0.0.0": {Value: uint(5), Expiration: futureExpiration}, - }, - Verified: map[string]CacheEntry{}, - Memory: map[string]uintptr{"rate": 8, "verified": 8}, - } - initialData, _ := json.Marshal(initialState) - if err := os.WriteFile(tmpFile, initialData, 0644); err != nil { - t.Fatalf("Failed to write initial state: %v", err) - } - - // Create caches with different data - rateCache := lru.New(1*time.Hour, 1*time.Minute) - verifiedCache := lru.New(1*time.Hour, 1*time.Minute) - - rateCache.Set("192.168.0.0", uint(10), lru.DefaultExpiration) - - // Save with reconciliation enabled - metrics, err := SaveStateToFileWithMetrics( - tmpFile, - true, // enable reconciliation - rateCache, - verifiedCache, - testLogger(), - ) - - if err != nil { - t.Fatalf("SaveStateToFile with reconciliation failed: %v", err) - } - - // Verify timing metrics (all should be non-negative) - if metrics.LockMs < 0 { - t.Error("Expected non-negative lockMs") - } - if metrics.ReadMs < 0 { - t.Error("Expected non-negative readMs when reconciliation is enabled") - } - if metrics.ReconcileMs < 0 { - t.Error("Expected non-negative reconcileMs when reconciliation is enabled") - } - if metrics.MarshalMs < 0 { - t.Error("Expected non-negative marshalMs") - } - if metrics.WriteMs < 0 { - t.Error("Expected non-negative writeMs") - } - if metrics.TotalMs < 0 { - t.Error("Expected non-negative totalMs") - } - - // Verify both entries are in the saved file (reconciled) - savedData, _ := os.ReadFile(tmpFile) - var savedState State - err = json.Unmarshal(savedData, &savedState) - if err != nil { - t.Errorf("Unable to unmarshal state %v", err) - } - - if len(savedState.Rate) != 2 { - t.Errorf("Expected 2 rate entries after reconciliation, got %d", len(savedState.Rate)) - } - }) - t.Run("Save with metrics", func(t *testing.T) { tmpFile := t.TempDir() + "/state.json" - - rateCache := lru.New(1*time.Hour, 1*time.Minute) verifiedCache := lru.New(1*time.Hour, 1*time.Minute) - - rateCache.Set("192.168.0.0", uint(10), lru.DefaultExpiration) verifiedCache.Set("5.6.7.8", true, lru.DefaultExpiration) metrics, err := SaveStateToFileWithMetrics( tmpFile, - false, - rateCache, verifiedCache, testLogger(), ) @@ -359,9 +139,6 @@ func TestSaveStateToFileWithMetrics(t *testing.T) { t.Fatalf("SaveStateToFileWithMetrics failed: %v", err) } - if metrics.RateEntries != 1 { - t.Errorf("Expected 1 rate entry, got %d", metrics.RateEntries) - } if metrics.VerifiedEntries != 1 { t.Errorf("Expected 1 verified entry, got %d", metrics.VerifiedEntries) } @@ -371,20 +148,14 @@ func TestSaveStateToFileWithMetrics(t *testing.T) { }) t.Run("File write error", func(t *testing.T) { - // Use invalid path to trigger error invalidPath := "/invalid/directory/that/does/not/exist/state.json" - - rateCache := lru.New(1*time.Hour, 1*time.Minute) verifiedCache := lru.New(1*time.Hour, 1*time.Minute) _, err := SaveStateToFileWithMetrics( invalidPath, - false, - rateCache, verifiedCache, testLogger(), ) - if err == nil { t.Error("Expected error for invalid file path, got nil") } @@ -395,18 +166,13 @@ func TestLoadStateFromFile(t *testing.T) { t.Run("Load valid state file", func(t *testing.T) { tmpFile := t.TempDir() + "/state.json" - // Create state file now := time.Now().UnixNano() futureExpiration := now + int64(1*time.Hour) - testState := State{ - Rate: map[string]CacheEntry{ - "192.168.0.0": {Value: uint(10), Expiration: futureExpiration}, - "10.0.0.0": {Value: uint(5), Expiration: futureExpiration}, - }, - Verified: map[string]CacheEntry{ + testState := map[string]interface{}{ + "verified": map[string]CacheEntry{ "5.6.7.8": {Value: true, Expiration: futureExpiration}, }, - Memory: map[string]uintptr{"rate": 8, "verified": 8}, + "memory": map[string]uintptr{"verified": 8}, } data, _ := json.Marshal(testState) @@ -414,27 +180,14 @@ func TestLoadStateFromFile(t *testing.T) { t.Fatalf("Failed to write test state: %v", err) } - // Load into empty caches - rateCache := lru.New(1*time.Hour, 1*time.Minute) verifiedCache := lru.New(1*time.Hour, 1*time.Minute) - - err := LoadStateFromFile(tmpFile, rateCache, verifiedCache) - if err != nil { + if err := LoadStateFromFile(tmpFile, verifiedCache); err != nil { t.Fatalf("LoadStateFromFile failed: %v", err) } - // Verify caches were populated - if rateCache.ItemCount() != 2 { - t.Errorf("Expected 2 rate entries, got %d", rateCache.ItemCount()) - } if verifiedCache.ItemCount() != 1 { t.Errorf("Expected 1 verified entry, got %d", verifiedCache.ItemCount()) } - - // Verify specific values - if v, ok := rateCache.Get("192.168.0.0"); !ok || v.(uint) != 10 { - t.Error("Expected rate 10 for 192.168.0.0") - } if v, ok := verifiedCache.Get("5.6.7.8"); !ok || v.(bool) != true { t.Error("Expected 5.6.7.8 to be verified") } @@ -443,182 +196,104 @@ func TestLoadStateFromFile(t *testing.T) { t.Run("Load expired entries", func(t *testing.T) { tmpFile := t.TempDir() + "/state.json" - // Create state with expired entries now := time.Now().UnixNano() pastExpiration := now - int64(1*time.Hour) testState := State{ - Rate: map[string]CacheEntry{ - "192.168.0.0": {Value: uint(10), Expiration: pastExpiration}, // expired + Verified: map[string]CacheEntry{ + "5.6.7.8": {Value: true, Expiration: pastExpiration}, }, - Verified: map[string]CacheEntry{}, - Memory: map[string]uintptr{"rate": 8, "verified": 8}, + Memory: map[string]uintptr{"verified": 8}, } data, _ := json.Marshal(testState) - err := os.WriteFile(tmpFile, data, 0644) - if err != nil { + if err := os.WriteFile(tmpFile, data, 0644); err != nil { t.Fatalf("Unable to write file: %v", err) } - // Load into empty caches - rateCache := lru.New(1*time.Hour, 1*time.Minute) - verifiedCache := lru.New(1*time.Hour, 1*time.Minute) - err = LoadStateFromFile(tmpFile, rateCache, verifiedCache) - if err != nil { + verifiedCache := lru.New(1*time.Hour, 1*time.Minute) + if err := LoadStateFromFile(tmpFile, verifiedCache); err != nil { t.Fatalf("LoadStateFromFile failed: %v", err) } - - // Expired entries should be filtered out - if rateCache.ItemCount() != 0 { - t.Errorf("Expected 0 entries (expired filtered out), got %d", rateCache.ItemCount()) + if verifiedCache.ItemCount() != 0 { + t.Errorf("Expected 0 entries (expired filtered out), got %d", verifiedCache.ItemCount()) } }) t.Run("File does not exist", func(t *testing.T) { nonExistentFile := t.TempDir() + "/does-not-exist.json" - - rateCache := lru.New(1*time.Hour, 1*time.Minute) verifiedCache := lru.New(1*time.Hour, 1*time.Minute) - err := LoadStateFromFile(nonExistentFile, rateCache, verifiedCache) - if err == nil { + if err := LoadStateFromFile(nonExistentFile, verifiedCache); err == nil { t.Error("Expected error for non-existent file, got nil") } }) t.Run("Invalid JSON", func(t *testing.T) { tmpFile := t.TempDir() + "/invalid.json" - - // Write invalid JSON if err := os.WriteFile(tmpFile, []byte(`{invalid json`), 0644); err != nil { t.Fatalf("Failed to write invalid JSON: %v", err) } - rateCache := lru.New(1*time.Hour, 1*time.Minute) verifiedCache := lru.New(1*time.Hour, 1*time.Minute) - - err := LoadStateFromFile(tmpFile, rateCache, verifiedCache) - if err == nil { + if err := LoadStateFromFile(tmpFile, verifiedCache); err == nil { t.Error("Expected error for invalid JSON, got nil") } - - // Caches should remain empty - if rateCache.ItemCount() != 0 { + if verifiedCache.ItemCount() != 0 { t.Error("Expected empty cache after failed load") } }) t.Run("Empty file", func(t *testing.T) { tmpFile := t.TempDir() + "/empty.json" - - // Write empty file if err := os.WriteFile(tmpFile, []byte{}, 0644); err != nil { t.Fatalf("Failed to write empty file: %v", err) } - rateCache := lru.New(1*time.Hour, 1*time.Minute) verifiedCache := lru.New(1*time.Hour, 1*time.Minute) - - // Empty file returns nil (no state to load, which is fine) - err := LoadStateFromFile(tmpFile, rateCache, verifiedCache) - if err != nil { + if err := LoadStateFromFile(tmpFile, verifiedCache); err != nil { t.Errorf("Unexpected error for empty file: %v", err) } - - // Caches should remain empty - if rateCache.ItemCount() != 0 { + if verifiedCache.ItemCount() != 0 { t.Error("Expected empty cache after loading empty file") } }) } -func TestReconcileStateFromFile(t *testing.T) { - tmpFile := t.TempDir() + "/state.json" - now := time.Now().UnixNano() - futureExpiration := now + int64(1*time.Hour) - laterExpiration := now + int64(2*time.Hour) - - fileState := State{ - Rate: map[string]CacheEntry{ - "192.168.0.0": {Value: uint(10), Expiration: futureExpiration}, - }, - Verified: map[string]CacheEntry{}, - Memory: map[string]uintptr{"rate": 8, "verified": 8}, - } - fileState.Verified = map[string]CacheEntry{ - "5.6.7.8": {Value: true, Expiration: futureExpiration}, - "9.9.9.9": {Value: true, Expiration: futureExpiration}, - } - data, _ := json.Marshal(fileState) - if err := os.WriteFile(tmpFile, data, 0644); err != nil { - t.Fatalf("Failed to write test state: %v", err) - } - - rateCache := lru.New(1*time.Hour, 1*time.Minute) - verifiedCache := lru.New(1*time.Hour, 1*time.Minute) - - rateCache.Set("10.0.0.0", uint(5), lru.DefaultExpiration) - verifiedCache.Set("9.9.9.9", false, time.Duration(laterExpiration-now)) - - if err := ReconcileStateFromFile(tmpFile, rateCache, verifiedCache); err != nil { - t.Fatalf("ReconcileStateFromFile failed: %v", err) - } - - if v, ok := rateCache.Get("192.168.0.0"); !ok || v.(uint) != 10 { - t.Error("Expected file state to be reconciled into memory") - } - if v, ok := rateCache.Get("10.0.0.0"); !ok || v.(uint) != 5 { - t.Error("Expected existing memory state to be retained") - } - if v, ok := verifiedCache.Get("5.6.7.8"); !ok || v.(bool) != true { - t.Error("Expected file verified state to be reconciled into memory") - } - if v, ok := verifiedCache.Get("9.9.9.9"); !ok || v.(bool) != false { - t.Error("Expected newer memory verified state to be retained") - } -} - func TestSaveStateToFileWithMetricsWriteError(t *testing.T) { statePath := t.TempDir() - rateCache := lru.New(1*time.Hour, 1*time.Minute) verifiedCache := lru.New(1*time.Hour, 1*time.Minute) - rateCache.Set("192.168.0.0", uint(10), lru.DefaultExpiration) + verifiedCache.Set("5.6.7.8", true, lru.DefaultExpiration) metrics, err := SaveStateToFileWithMetrics( statePath, - false, - rateCache, verifiedCache, testLogger(), ) if err == nil { t.Fatal("expected write error when state path is a directory") } - if metrics.RateEntries != 1 { - t.Fatalf("expected metrics to include marshaled rate entry, got %d", metrics.RateEntries) + if metrics.VerifiedEntries != 1 { + t.Fatalf("expected metrics to include marshaled verified entry, got %d", metrics.VerifiedEntries) } } func TestAtomicWriteStateFileCreateTempError(t *testing.T) { missingDir := filepath.Join(t.TempDir(), "missing") - _, _, err := atomicWriteStateFile(filepath.Join(missingDir, "state.json"), nil, nil, 0600) + _, _, err := atomicWriteStateFile(filepath.Join(missingDir, "state.json"), nil, 0600) if err == nil { t.Fatal("expected atomicWriteStateFile to fail when temp directory is missing") } } func TestWriteStateJSON(t *testing.T) { - rateCache := lru.New(1*time.Hour, 1*time.Minute) verifiedCache := lru.New(1*time.Hour, 1*time.Minute) - - rateCache.Set("192.168.0.0", uint(10), lru.DefaultExpiration) - rateCache.Set("bad\a-key", uint(2), lru.DefaultExpiration) verifiedCache.Set("9.9.9.9", true, lru.DefaultExpiration) + verifiedCache.Set("bad\a-key", true, lru.DefaultExpiration) var buf bytes.Buffer writer := bufio.NewWriter(&buf) - if err := writeStateJSON(writer, rateCache.Items(), verifiedCache.Items()); err != nil { + if err := writeStateJSON(writer, verifiedCache.Items()); err != nil { t.Fatalf("writeStateJSON failed: %v", err) } if err := writer.Flush(); err != nil { @@ -629,11 +304,11 @@ func TestWriteStateJSON(t *testing.T) { if err := json.Unmarshal(buf.Bytes(), &saved); err != nil { t.Fatalf("state JSON did not unmarshal: %v", err) } - if len(saved.Rate) != 2 || len(saved.Verified) != 1 { - t.Fatalf("unexpected saved counts: rate=%d verified=%d", len(saved.Rate), len(saved.Verified)) + if len(saved.Verified) != 2 { + t.Fatalf("unexpected verified count: %d", len(saved.Verified)) } - if saved.Rate["bad\a-key"].Value != float64(2) { - t.Fatal("expected JSON-escaped rate key to round-trip") + if saved.Verified["bad\a-key"].Value != true { + t.Fatal("expected JSON-escaped verified key to round-trip") } } @@ -642,362 +317,91 @@ func TestWriteStateJSONUnexpectedType(t *testing.T) { writer := bufio.NewWriter(&buf) err := writeStateJSON( writer, - map[string]lru.Item{"bad": {Object: "not-a-uint", Expiration: time.Now().Add(time.Hour).UnixNano()}}, - nil, + map[string]lru.Item{"bad": {Object: "not-a-bool", Expiration: time.Now().Add(time.Hour).UnixNano()}}, ) if err == nil { t.Fatal("expected writeStateJSON to reject unexpected cache value type") } } -func TestReconcileStateFromFileEmptyAndInvalidFiles(t *testing.T) { - newCaches := func() (*lru.Cache, *lru.Cache) { - return lru.New(1*time.Hour, 1*time.Minute), - lru.New(1*time.Hour, 1*time.Minute) - } - - t.Run("missing file", func(t *testing.T) { - rateCache, verifiedCache := newCaches() - err := ReconcileStateFromFile(filepath.Join(t.TempDir(), "missing.json"), rateCache, verifiedCache) - if err == nil { - t.Fatal("expected missing state file to return read error") - } - }) - - t.Run("empty file", func(t *testing.T) { - tmpFile := filepath.Join(t.TempDir(), "state.json") - if err := os.WriteFile(tmpFile, nil, 0600); err != nil { - t.Fatalf("failed to write empty state file: %v", err) - } - - rateCache, verifiedCache := newCaches() - if err := ReconcileStateFromFile(tmpFile, rateCache, verifiedCache); err != nil { - t.Fatalf("empty state file should be a no-op: %v", err) - } - }) - - t.Run("invalid JSON", func(t *testing.T) { - tmpFile := filepath.Join(t.TempDir(), "state.json") - if err := os.WriteFile(tmpFile, []byte("{invalid json"), 0600); err != nil { - t.Fatalf("failed to write invalid state file: %v", err) - } - - rateCache, verifiedCache := newCaches() - if err := ReconcileStateFromFile(tmpFile, rateCache, verifiedCache); err == nil { - t.Fatal("expected invalid state file to return unmarshal error") - } - }) -} - func testLogger() *slog.Logger { return slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{ Level: slog.LevelError, // Only show errors during tests })) } -// TestSetStateWithExpiration_Synctest uses synctest to verify expiration logic func TestSetStateWithExpiration_Synctest(t *testing.T) { synctest.Test(t, func(t *testing.T) { - // Initial time: midnight UTC 2000-01-01 start := time.Now() - // Create state with entries expiring at different times state := State{ - Rate: map[string]CacheEntry{ - "192.168.0.0": { - Value: uint(10), - Expiration: start.Add(5 * time.Second).UnixNano(), // expires in 5s - }, - "10.0.0.0": { - Value: uint(5), - Expiration: start.Add(10 * time.Second).UnixNano(), // expires in 10s - }, - }, Verified: map[string]CacheEntry{ "9.9.9.9": { Value: true, - Expiration: 0, // never expires + Expiration: 0, }, - }, - } - - // Create empty caches (no cleanup interval to avoid background goroutines) - rateCache := lru.New(1*time.Hour, lru.NoExpiration) - verifiedCache := lru.New(1*time.Hour, lru.NoExpiration) - - // Load state - SetState(state, rateCache, verifiedCache) - - // Verify all entries are loaded - if rateCache.ItemCount() != 2 { - t.Errorf("Expected 2 rate entries, got %d", rateCache.ItemCount()) - } - if verifiedCache.ItemCount() != 1 { - t.Errorf("Expected 1 verified entry, got %d", verifiedCache.ItemCount()) - } - - // Advance time by 4 seconds (rate entries still valid) - time.Sleep(4 * time.Second) - synctest.Wait() - - // Rate entries should still be present - if _, found := rateCache.Get("192.168.0.0"); !found { - t.Error("Rate entry 192.168.0.0 should not expire until 5 seconds") - } - if _, found := rateCache.Get("10.0.0.0"); !found { - t.Error("Rate entry 10.0.0.0 should not expire until 10 seconds") - } - - // Advance time by 2 more seconds (total 6s, first rate entry should expire) - time.Sleep(2 * time.Second) - synctest.Wait() - - // First rate entry should be expired - if _, found := rateCache.Get("192.168.0.0"); found { - t.Error("Rate entry 192.168.0.0 should have expired after 5 seconds") - } - - // Second rate entry should still be present - if _, found := rateCache.Get("10.0.0.0"); !found { - t.Error("Rate entry 10.0.0.0 should not expire until 10 seconds") - } - - // Verified entry with no expiration should still be present - if _, found := verifiedCache.Get("9.9.9.9"); !found { - t.Error("Verified entry with no expiration should never expire") - } - - // Advance time by 5 more seconds (total 11s, all time-based entries expired) - time.Sleep(5 * time.Second) - synctest.Wait() - - // All time-based entries should be expired - if _, found := rateCache.Get("10.0.0.0"); found { - t.Error("Rate entry 10.0.0.0 should have expired after 10 seconds") - } - - // Only the never-expiring verified entry should remain - if _, found := verifiedCache.Get("9.9.9.9"); !found { - t.Error("Verified entry with no expiration should still be present after 11 seconds") - } - }) -} - -// TestReconcileStateWithExpiration_Synctest tests reconciliation with time control -func TestReconcileStateWithExpiration_Synctest(t *testing.T) { - synctest.Test(t, func(t *testing.T) { - start := time.Now() - - // Create file state with entries expiring at different times - fileState := State{ - Rate: map[string]CacheEntry{ - "192.168.0.0": { - Value: uint(15), - Expiration: start.Add(10 * time.Second).UnixNano(), // newer expiration - }, - "10.0.0.0": { - Value: uint(3), - Expiration: start.Add(5 * time.Second).UnixNano(), // older expiration + "8.8.8.8": { + Value: true, + Expiration: start.Add(5 * time.Second).UnixNano(), }, }, } - // Create memory caches with overlapping data (no cleanup interval to avoid background goroutines) - rateCache := lru.New(1*time.Hour, lru.NoExpiration) verifiedCache := lru.New(1*time.Hour, lru.NoExpiration) + SetState(state, verifiedCache) - // Memory entry with older expiration (should be replaced) - rateCache.Set("192.168.0.0", uint(10), 5*time.Second) - // Memory entry with newer expiration (should be kept) - rateCache.Set("10.0.0.0", uint(5), 10*time.Second) - - // Reconcile - ReconcileState(fileState, rateCache, verifiedCache) - - // 192.168.0.0 should have file's value (newer expiration) - if v, ok := rateCache.Get("192.168.0.0"); !ok || v.(uint) != 15 { - t.Errorf("Expected rate 15 for 192.168.0.0, got %v", v) + if verifiedCache.ItemCount() != 2 { + t.Errorf("Expected 2 verified entries, got %d", verifiedCache.ItemCount()) } - // 10.0.0.0 should have memory's value (newer expiration) - if v, ok := rateCache.Get("10.0.0.0"); !ok || v.(uint) != 5 { - t.Errorf("Expected rate 5 for 10.0.0.0 (memory kept), got %v", v) - } - - // Advance time by 6 seconds time.Sleep(6 * time.Second) synctest.Wait() - // Both entries should still be present (both have 10s expiration from reconciliation) - // - 192.168.0.0 has file's value (15) with 10s expiration - // - 10.0.0.0 has memory's value (5) with 10s expiration - if _, found := rateCache.Get("10.0.0.0"); !found { - t.Error("Entry 10.0.0.0 should not expire until 10 seconds (memory had newer expiration)") - } - - if _, found := rateCache.Get("192.168.0.0"); !found { - t.Error("Entry 192.168.0.0 should not expire until 10 seconds (file had newer expiration)") - } - - // Advance time by 5 more seconds (total 11s) - time.Sleep(5 * time.Second) - synctest.Wait() - - // All entries should be expired (verify by trying to get them) - if _, found := rateCache.Get("192.168.0.0"); found { - t.Error("Entry 192.168.0.0 should have expired after 10 seconds") + if _, found := verifiedCache.Get("8.8.8.8"); found { + t.Error("Verified entry 8.8.8.8 should have expired after 5 seconds") } - // Manually trigger cleanup since we're not using automatic janitor - rateCache.DeleteExpired() - if rateCache.ItemCount() != 0 { - t.Errorf("Expected all entries expired, got %d entries", rateCache.ItemCount()) + if _, found := verifiedCache.Get("9.9.9.9"); !found { + t.Error("Verified entry with no expiration should still be present") } }) } -// TestSaveAndLoadStateWithExpiration_Synctest tests full save/load cycle with time control func TestSaveAndLoadStateWithExpiration_Synctest(t *testing.T) { synctest.Test(t, func(t *testing.T) { tmpFile := t.TempDir() + "/state.json" - // Create caches with entries expiring at different times (no cleanup interval to avoid background goroutines) - rateCache1 := lru.New(1*time.Hour, lru.NoExpiration) verifiedCache1 := lru.New(1*time.Hour, lru.NoExpiration) - - rateCache1.Set("192.168.0.0", uint(10), 5*time.Second) - rateCache1.Set("10.0.0.0", uint(5), 10*time.Second) + verifiedCache1.Set("8.8.8.8", true, 5*time.Second) verifiedCache1.Set("9.9.9.9", true, lru.NoExpiration) - // Save state - _, err := SaveStateToFileWithMetrics( + if _, err := SaveStateToFileWithMetrics( tmpFile, - false, - rateCache1, verifiedCache1, testLogger(), - ) - if err != nil { + ); err != nil { t.Fatalf("SaveStateToFile failed: %v", err) } - // Advance time by 4 seconds (rates still valid) time.Sleep(4 * time.Second) synctest.Wait() - // Load into new caches (no cleanup interval to avoid background goroutines) - rateCache2 := lru.New(1*time.Hour, lru.NoExpiration) verifiedCache2 := lru.New(1*time.Hour, lru.NoExpiration) - - err = LoadStateFromFile(tmpFile, rateCache2, verifiedCache2) - if err != nil { + if err := LoadStateFromFile(tmpFile, verifiedCache2); err != nil { t.Fatalf("LoadStateFromFile failed: %v", err) } - // First rate entry should be loaded (expires at 5s, we're at 4s) - if _, found := rateCache2.Get("192.168.0.0"); !found { - t.Error("Rate entry 192.168.0.0 should be loaded (not yet expired)") - } - - // Second rate entry should be loaded (expires at 10s, we're at 4s) - if _, found := rateCache2.Get("10.0.0.0"); !found { - t.Error("Rate entry 10.0.0.0 should be loaded (not yet expired)") + if _, found := verifiedCache2.Get("8.8.8.8"); !found { + t.Error("Verified entry 8.8.8.8 should be loaded before expiration") } - - // Verified entry should be loaded (no expiration) if _, found := verifiedCache2.Get("9.9.9.9"); !found { - t.Error("Verified entry should be loaded (no expiration)") + t.Error("Verified entry with no expiration should be loaded") } - // Advance time by 2 more seconds (total 6s, first rate entry expires) time.Sleep(2 * time.Second) synctest.Wait() - // First rate entry should be expired - if _, found := rateCache2.Get("192.168.0.0"); found { - t.Error("Rate entry 192.168.0.0 should have expired") - } - - // Second rate entry should still exist - if _, found := rateCache2.Get("10.0.0.0"); !found { - t.Error("Rate entry 10.0.0.0 should still be present") - } - }) -} - -// TestReconcilePreservesNewerData_Synctest verifies reconciliation keeps fresher data -func TestReconcilePreservesNewerData_Synctest(t *testing.T) { - synctest.Test(t, func(t *testing.T) { - tmpFile := t.TempDir() + "/state.json" - - // Create initial state file with data expiring in 5 seconds (no cleanup interval to avoid background goroutines) - initialCache := lru.New(1*time.Hour, lru.NoExpiration) - initialCache.Set("192.168.0.0", uint(100), 5*time.Second) - - _, err := SaveStateToFileWithMetrics( - tmpFile, - false, - initialCache, - lru.New(1*time.Hour, lru.NoExpiration), - testLogger(), - ) - if err != nil { - t.Fatalf("Initial save failed: %v", err) - } - - // Advance time by 2 seconds - time.Sleep(2 * time.Second) - synctest.Wait() - - // Create new in-memory data with expiration in 10 seconds from original start - // This represents fresher data (no cleanup interval to avoid background goroutines) - newCache := lru.New(1*time.Hour, lru.NoExpiration) - newCache.Set("192.168.0.0", uint(200), 8*time.Second) // expires at start+10s - - // Save with reconciliation enabled - _, err = SaveStateToFileWithMetrics( - tmpFile, - true, // reconcile - newCache, - lru.New(1*time.Hour, lru.NoExpiration), - testLogger(), - ) - if err != nil { - t.Fatalf("Reconciled save failed: %v", err) - } - - // Load back and verify we got the newer value (no cleanup interval to avoid background goroutines) - loadedCache := lru.New(1*time.Hour, lru.NoExpiration) - err = LoadStateFromFile( - tmpFile, - loadedCache, - lru.New(1*time.Hour, lru.NoExpiration), - ) - if err != nil { - t.Fatalf("Load failed: %v", err) - } - - // Should have the newer value (200 with later expiration) - if v, found := loadedCache.Get("192.168.0.0"); !found || v.(uint) != 200 { - t.Errorf("Expected value 200 (newer data), got %v (found=%v)", v, found) - } - - // Advance time by 4 more seconds (total 6s from start) - // Old data would have expired at 5s, new data expires at 10s - time.Sleep(4 * time.Second) - synctest.Wait() - - // New data should still be valid - if _, found := loadedCache.Get("192.168.0.0"); !found { - t.Error("Newer data should still be valid (expires at 10s, we're at 6s)") - } - - // Advance time by 5 more seconds (total 11s from start) - time.Sleep(5 * time.Second) - synctest.Wait() - - // Now the newer data should also be expired - if _, found := loadedCache.Get("192.168.0.0"); found { - t.Error("Newer data should have expired after 10 seconds") + if _, found := verifiedCache2.Get("8.8.8.8"); found { + t.Error("Verified entry 8.8.8.8 should have expired") } }) } @@ -1007,33 +411,25 @@ func TestReconcilePreservesNewerData_Synctest(t *testing.T) { func TestCacheCleanupInterval_Synctest(t *testing.T) { t.Skip("Skipping test that requires janitor goroutine (incompatible with synctest)") synctest.Test(t, func(t *testing.T) { - // Create cache with 1 minute cleanup interval cleanupInterval := 1 * time.Minute cache := lru.New(5*time.Second, cleanupInterval) - // Add entry that expires in 3 seconds cache.Set("test-key", uint(42), 3*time.Second) - // Verify entry exists if _, found := cache.Get("test-key"); !found { t.Fatal("Entry should exist immediately after Set") } - // Advance time by 4 seconds (entry expired but cleanup hasn't run) time.Sleep(4 * time.Second) synctest.Wait() - // Entry is expired but might still be in cache (cleanup hasn't run yet) - // The Get should return false because go-cache checks expiration on Get if _, found := cache.Get("test-key"); found { t.Error("Entry should be expired after 3 seconds") } - // Advance time to trigger cleanup (cleanup runs every 1 minute) - time.Sleep(57 * time.Second) // Total 61 seconds, cleanup should have run + time.Sleep(57 * time.Second) synctest.Wait() - // Entry should definitely be cleaned up now if cache.ItemCount() != 0 { t.Errorf("Cache should be empty after cleanup, got %d items", cache.ItemCount()) } diff --git a/main.go b/main.go index 41c410c..5be2db1 100644 --- a/main.go +++ b/main.go @@ -30,8 +30,6 @@ import ( const ( // StateSaveInterval is how often local persistent state is written to disk. StateSaveInterval = 60 * time.Second - // StateReconciliationSaveInterval is the faster save cadence used when multiple instances share state. - StateReconciliationSaveInterval = 10 * time.Second // StateSaveJitter is the maximum random jitter added to save interval to prevent thundering herd StateSaveJitter = 2 * time.Second @@ -50,10 +48,7 @@ const ( ) type Config struct { - RateLimit uint `json:"rateLimit"` Window int64 `json:"window"` - IPv4SubnetMask int `json:"ipv4subnetMask"` - IPv6SubnetMask int `json:"ipv6subnetMask"` IPForwardedHeader string `json:"ipForwardedHeader"` IPDepth int `json:"ipDepth"` // ProtectParameters is a string instead of bool due to Traefik's label parsing limitations @@ -72,20 +67,14 @@ type Config struct { SiteKey string `json:"siteKey"` SecretKey string `json:"secretKey"` // EnableStatsPage is a string instead of bool due to Traefik's label parsing limitations - EnableStatsPage string `json:"enableStatsPage"` - LogLevel string `json:"loglevel,omitempty"` - PersistentStateFile string `json:"persistentStateFile"` - // EnableStateReconciliation is a string instead of bool due to Traefik's label parsing limitations - // When enabled, the plugin will read and merge state from disk before each save to prevent - // multiple instances from overwriting each other's data. This adds extra I/O overhead. - // Only enable this if running multiple plugin instances sharing the same state file. - // Performance warning: Not recommended for sites with >1M unique visitors (see internal/state/state_stress_test.go). - EnableStateReconciliation string `json:"enableStateReconciliation"` - EnableGooglebotIPCheck string `json:"enableGooglebotIPCheck"` - EnableUptimeRobotBypass string `json:"enableUptimeRobotBypass"` - Mode string `json:"mode"` - PeriodSeconds int `json:"periodSeconds"` - FailureThreshold int `json:"failureThreshold"` + EnableStatsPage string `json:"enableStatsPage"` + LogLevel string `json:"loglevel,omitempty"` + PersistentStateFile string `json:"persistentStateFile"` + EnableGooglebotIPCheck string `json:"enableGooglebotIPCheck"` + EnableUptimeRobotBypass string `json:"enableUptimeRobotBypass"` + Mode string `json:"mode"` + PeriodSeconds int `json:"periodSeconds"` + FailureThreshold int `json:"failureThreshold"` } type CaptchaProtect struct { @@ -94,7 +83,6 @@ type CaptchaProtect struct { config *Config log *slog.Logger httpClient *http.Client - rateCache *lru.Cache verifiedCache *lru.Cache botCache *lru.Cache googlebotIPs *helper.GooglebotIPs @@ -102,14 +90,11 @@ type CaptchaProtect struct { captchaConfig CaptchaConfig exemptIps []*net.IPNet tmpl *htemplate.Template - ipv4Mask net.IPMask - ipv6Mask net.IPMask protectRoutesRegex []*regexp.Regexp excludeRoutesRegex []*regexp.Regexp stateMu sync.Mutex stateDirty uint64 stateSavedDirty uint64 - stateFileModTime time.Time goodBotLookup func(context.Context, string, []string) bool // Circuit breaker fields @@ -141,32 +126,28 @@ type challengeData struct { func CreateConfig() *Config { return &Config{ - RateLimit: 20, - Window: 86400, - IPv4SubnetMask: 16, - IPv6SubnetMask: 64, - IPForwardedHeader: "", - ProtectParameters: "false", - ProtectRoutes: []string{}, - ExcludeRoutes: []string{}, - ProtectHttpMethods: []string{}, - ProtectFileExtensions: []string{}, - GoodBots: []string{}, - ExemptIPs: []string{}, - ExemptUserAgents: []string{}, - ChallengeURL: "/challenge", - ChallengeTmpl: "challenge.tmpl.html", - ChallengeStatusCode: 0, - EnableStatsPage: "false", - LogLevel: "INFO", - IPDepth: 0, - CaptchaProvider: "turnstile", - Mode: "prefix", - EnableStateReconciliation: "false", - EnableGooglebotIPCheck: "false", - EnableUptimeRobotBypass: "false", - PeriodSeconds: DefaultHealthCheckPeriodSeconds, - FailureThreshold: DefaultHealthCheckFailureThreshold, + Window: 86400, + IPForwardedHeader: "", + ProtectParameters: "false", + ProtectRoutes: []string{}, + ExcludeRoutes: []string{}, + ProtectHttpMethods: []string{}, + ProtectFileExtensions: []string{}, + GoodBots: []string{}, + ExemptIPs: []string{}, + ExemptUserAgents: []string{}, + ChallengeURL: "/challenge", + ChallengeTmpl: "challenge.tmpl.html", + ChallengeStatusCode: 0, + EnableStatsPage: "false", + LogLevel: "INFO", + IPDepth: 0, + CaptchaProvider: "turnstile", + Mode: "prefix", + EnableGooglebotIPCheck: "false", + EnableUptimeRobotBypass: "false", + PeriodSeconds: DefaultHealthCheckPeriodSeconds, + FailureThreshold: DefaultHealthCheckFailureThreshold, } } @@ -232,10 +213,10 @@ func NewCaptchaProtect(ctx context.Context, next http.Handler, config *Config, n config.ExemptUserAgents = ua if config.ChallengeURL == "/" { - return nil, fmt.Errorf("your challenge URL can not be the entire site. Default is `/challenge`. A blank value will have challenges presented on the visit that trips the rate limit") + return nil, fmt.Errorf("your challenge URL can not be the entire site. Default is `/challenge`. A blank value will have challenges presented on the visit that triggers protection") } - // when challenging on the same page that tripped the rate limiter + // when challenging on the same page that triggered protection // add a url parameter to detect on if config.ChallengeURL == "" { config.ChallengeURL = "?challenge=true" @@ -298,7 +279,6 @@ func NewCaptchaProtect(ctx context.Context, next http.Handler, config *Config, n httpClient: &http.Client{ Timeout: 10 * time.Second, }, - rateCache: lru.New(expiration, 1*time.Minute), botCache: lru.New(expiration, 1*time.Hour), goodBotLookup: helper.IsIpGoodBotContext, verifiedCache: lru.New(expiration, 1*time.Hour), @@ -317,16 +297,6 @@ func NewCaptchaProtect(ctx context.Context, next http.Handler, config *Config, n } } - err := bc.SetIpv4Mask(config.IPv4SubnetMask) - if err != nil { - return nil, err - } - - err = bc.SetIpv6Mask(config.IPv6SubnetMask) - if err != nil { - return nil, err - } - // set the captcha config based on the provider // thanks to https://github.com/maxlerebourg/crowdsec-bouncer-traefik-plugin/blob/4708d76854c7ae95fa7313c46fbe21959be2fff1/pkg/captcha/captcha.go#L39-L55 // for the struct/idea @@ -577,7 +547,7 @@ func (bc *CaptchaProtect) recordHealthCheckFailure() { } func (bc *CaptchaProtect) ServeHTTP(rw http.ResponseWriter, req *http.Request) { - clientIP, ipRange := bc.getClientIP(req) + clientIP := bc.getClientIP(req) // Serve proof-of-javascript JS if req.URL.Path == "/captcha-protect-poj.js" { @@ -615,12 +585,6 @@ func (bc *CaptchaProtect) ServeHTTP(rw http.ResponseWriter, req *http.Request) { bc.next.ServeHTTP(rw, req) return } - bc.registerRequest(ipRange) - - if !bc.trippedRateLimit(ipRange) { - bc.next.ServeHTTP(rw, req) - return - } encodedURI := url.QueryEscape(req.RequestURI) if bc.ChallengeOnPage() { @@ -843,7 +807,7 @@ func (bc *CaptchaProtect) serveStatsPage(rw http.ResponseWriter, ip string) { return } - state := state.GetState(bc.rateCache.Items(), bc.verifiedCache.Items()) + state := state.GetState(bc.verifiedCache.Items()) jsonData, err := json.Marshal(state) if err != nil { bc.log.Error("failed to marshal JSON", "err", err) @@ -988,36 +952,7 @@ protected: return false } -func (bc *CaptchaProtect) trippedRateLimit(ip string) bool { - v, ok := bc.rateCache.Get(ip) - if !ok { - bc.log.Error("IP not found, but should already be set", "ip", ip) - return false - } - return v.(uint) > bc.config.RateLimit -} - -func (bc *CaptchaProtect) registerRequest(ip string) { - err := bc.rateCache.Add(ip, uint(1), lru.DefaultExpiration) - if err == nil { - bc.markStateDirty() - return - } - - v, ok := bc.rateCache.Get(ip) - if ok && v.(uint) > bc.config.RateLimit { - return - } - - _, err = bc.rateCache.IncrementUint(ip, uint(1)) - if err != nil { - bc.log.Error("unable to set rate cache", "ip", ip, "err", err) - return - } - bc.markStateDirty() -} - -func (bc *CaptchaProtect) getClientIP(req *http.Request) (string, string) { +func (bc *CaptchaProtect) getClientIP(req *http.Request) string { ip := req.Header.Get(bc.config.IPForwardedHeader) if bc.config.IPForwardedHeader != "" && ip != "" { components := strings.Split(ip, ",") @@ -1053,48 +988,7 @@ func (bc *CaptchaProtect) getClientIP(req *http.Request) (string, string) { } } - return bc.ParseIp(ip) -} - -func (bc *CaptchaProtect) ParseIp(ip string) (string, string) { - parsedIP := net.ParseIP(ip) - if parsedIP == nil { - return ip, ip - } - - // For IPv4 addresses - if parsedIP.To4() != nil { - subnet := parsedIP.Mask(bc.ipv4Mask) - return ip, subnet.String() - } - - // For IPv6 addresses - if parsedIP.To16() != nil { - subnet := parsedIP.Mask(bc.ipv6Mask) - return ip, subnet.String() - } - - bc.log.Warn("Unknown ip version", "ip", ip) - - return ip, ip -} - -func (bc *CaptchaProtect) SetIpv4Mask(m int) error { - if m < 8 || m > 32 { - return fmt.Errorf("invalid ipv4 mask: %d. Must be between 8 and 32", m) - } - bc.ipv4Mask = net.CIDRMask(m, 32) - - return nil -} - -func (bc *CaptchaProtect) SetIpv6Mask(m int) error { - if m < 8 || m > 128 { - return fmt.Errorf("invalid ipv6 mask: %d. Must be between 8 and 128", m) - } - bc.ipv6Mask = net.CIDRMask(m, 128) - - return nil + return ip } func (bc *CaptchaProtect) isGoodBot(req *http.Request, clientIP string) bool { @@ -1151,7 +1045,7 @@ func (c *Config) ParseHttpMethods(log *slog.Logger) { func (bc *CaptchaProtect) saveState(ctx context.Context) { // Add random jitter to prevent multiple instances from trying to save simultaneously jitter := stateSaveJitter() - baseInterval := stateSaveInterval(bc.config) + baseInterval := StateSaveInterval interval := baseInterval + jitter bc.log.Debug("State save configured", "baseInterval", baseInterval, "jitter", jitter, "actualInterval", interval) @@ -1169,16 +1063,11 @@ func (bc *CaptchaProtect) saveState(ctx context.Context) { bc.log.Error("unable to save state, could not close state file", "stateFile", bc.config.PersistentStateFile, "err", err) return } - bc.refreshStateFileModTime() - lastSave := time.Time{} for { select { case <-ticker.C: - if bc.config.EnableStateReconciliation == "true" { - bc.reconcileStateFromFileIfChanged() - } if !bc.hasUnsavedState() { continue } @@ -1191,9 +1080,6 @@ func (bc *CaptchaProtect) saveState(ctx context.Context) { } case <-ctx.Done(): - if bc.config.EnableStateReconciliation == "true" { - bc.reconcileStateFromFileIfChanged() - } if bc.hasUnsavedState() { bc.log.Debug("Context cancelled, running saveState before shutdown") bc.saveStateNow() @@ -1203,13 +1089,6 @@ func (bc *CaptchaProtect) saveState(ctx context.Context) { } } -func stateSaveInterval(config *Config) time.Duration { - if config.EnableStateReconciliation == "true" { - return StateReconciliationSaveInterval - } - return StateSaveInterval -} - func stateSaveJitter() time.Duration { maxJitter := big.NewInt(StateSaveJitter.Milliseconds()) if maxJitter.Sign() <= 0 { @@ -1240,13 +1119,10 @@ func fallbackStateSaveJitter(maxMillis int64) time.Duration { // saveStateNow performs an immediate state save using the state package. func (bc *CaptchaProtect) saveStateNow() bool { - reconcile := bc.config.EnableStateReconciliation == "true" dirtyAtStart := bc.currentStateDirty() metrics, err := state.SaveStateToFileWithMetrics( bc.config.PersistentStateFile, - reconcile, - bc.rateCache, bc.verifiedCache, bc.log, ) @@ -1256,14 +1132,10 @@ func (bc *CaptchaProtect) saveStateNow() bool { return false } bc.markStateSaved(dirtyAtStart) - bc.refreshStateFileModTime() bc.log.Debug("State saved successfully", - "rateEntries", metrics.RateEntries, "verifiedEntries", metrics.VerifiedEntries, "lockMs", metrics.LockMs, - "readMs", metrics.ReadMs, - "reconcileMs", metrics.ReconcileMs, "marshalMs", metrics.MarshalMs, "writeMs", metrics.WriteMs, "totalMs", metrics.TotalMs, @@ -1278,7 +1150,6 @@ func (bc *CaptchaProtect) loadState() { func (bc *CaptchaProtect) loadStateFrom(filePath string) { err := state.LoadStateFromFile( filePath, - bc.rateCache, bc.verifiedCache, ) @@ -1288,7 +1159,6 @@ func (bc *CaptchaProtect) loadStateFrom(filePath string) { } bc.log.Info("Loaded previous state") - bc.refreshStateFileModTimeFrom(filePath) } func (bc *CaptchaProtect) markStateDirty() { @@ -1330,52 +1200,6 @@ func (bc *CaptchaProtect) markStateSaved(dirty uint64) { bc.stateSavedDirty = dirty } -func (bc *CaptchaProtect) refreshStateFileModTime() { - bc.refreshStateFileModTimeFrom(bc.config.PersistentStateFile) -} - -func (bc *CaptchaProtect) refreshStateFileModTimeFrom(filePath string) { - info, err := os.Stat(filePath) - if err != nil { - return - } - bc.stateMu.Lock() - defer bc.stateMu.Unlock() - bc.stateFileModTime = info.ModTime() -} - -func (bc *CaptchaProtect) reconcileStateFromFileIfChanged() { - info, err := os.Stat(bc.config.PersistentStateFile) - if err != nil { - return - } - modTime := info.ModTime() - bc.stateMu.Lock() - lastModTime := bc.stateFileModTime - bc.stateMu.Unlock() - if !lastModTime.IsZero() && !modTime.After(lastModTime) { - return - } - - err = state.ReconcileStateFromFile( - bc.config.PersistentStateFile, - bc.rateCache, - bc.verifiedCache, - ) - if err != nil { - bc.log.Warn("failed to reconcile state file", "err", err) - return - } - - if info, err := os.Stat(bc.config.PersistentStateFile); err == nil { - modTime = info.ModTime() - } - bc.stateMu.Lock() - bc.stateFileModTime = modTime - bc.stateMu.Unlock() - bc.log.Debug("Reconciled newer state file") -} - func (bc *CaptchaProtect) ChallengeOnPage() bool { return bc.config.ChallengeURL == "?challenge=true" } diff --git a/main_test.go b/main_test.go index 2b69c73..86b1de6 100644 --- a/main_test.go +++ b/main_test.go @@ -19,100 +19,6 @@ import ( "github.com/libops/captcha-protect/internal/helper" ) -func TestParseIp(t *testing.T) { - tests := []struct { - name string - ip string - ipv4Mask int - ipv6Mask int - wantFull string - wantSubnet string - }{ - { - name: "IPv4 /8", - ip: "192.168.1.1", - ipv4Mask: 8, - wantFull: "192.168.1.1", - wantSubnet: "192.0.0.0", - }, - { - name: "IPv4 /10", - ip: "192.168.1.1", - ipv4Mask: 10, - wantFull: "192.168.1.1", - wantSubnet: "192.128.0.0", - }, - { - name: "IPv4 /16", - ip: "192.168.1.1", - ipv4Mask: 16, - wantFull: "192.168.1.1", - wantSubnet: "192.168.0.0", - }, - { - name: "IPv4 /20", - ip: "192.168.1.1", - ipv4Mask: 20, - wantFull: "192.168.1.1", - wantSubnet: "192.168.0.0", - }, - { - name: "IPv4 /24", - ip: "192.168.1.1", - ipv4Mask: 24, - wantFull: "192.168.1.1", - wantSubnet: "192.168.1.0", - }, - { - name: "IPv4 /32", - ip: "192.168.1.1", - ipv4Mask: 32, - wantFull: "192.168.1.1", - wantSubnet: "192.168.1.1", - }, - { - name: "IPv6 /64", - ip: "2001:0db8:85a3:1234:5678:8a2e:0370:7334", - ipv6Mask: 64, - wantFull: "2001:0db8:85a3:1234:5678:8a2e:0370:7334", - wantSubnet: "2001:db8:85a3:1234::", - }, - { - name: "IPv6 /48", - ip: "2001:0db8:85a3:1234:5678:8a2e:0370:7334", - ipv6Mask: 48, - wantFull: "2001:0db8:85a3:1234:5678:8a2e:0370:7334", - wantSubnet: "2001:db8:85a3::", - }, - { - name: "Invalid IP returns same string", - ip: "not.an.ip", - ipv4Mask: 16, - ipv6Mask: 64, - wantFull: "not.an.ip", - wantSubnet: "not.an.ip", - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - c := CreateConfig() - bc := &CaptchaProtect{ - config: c, - ipv4Mask: net.CIDRMask(tc.ipv4Mask, 32), - ipv6Mask: net.CIDRMask(tc.ipv6Mask, 128), - } - gotFull, gotSubnet := bc.ParseIp(tc.ip) - if gotFull != tc.wantFull { - t.Errorf("ParseIp(%q, %d, %d) got full = %q, want %q", tc.ip, tc.ipv4Mask, tc.ipv6Mask, gotFull, tc.wantFull) - } - if gotSubnet != tc.wantSubnet { - t.Errorf("ParseIp(%q, %d, %d) got subnet = %q, want %q", tc.ip, tc.ipv4Mask, tc.ipv6Mask, gotSubnet, tc.wantSubnet) - } - }) - } -} - func TestRouteIsProtected(t *testing.T) { tests := []struct { name string @@ -464,7 +370,7 @@ func TestGetClientIP(t *testing.T) { t.Errorf("unexpected error %v", err) } - ip, _ := bc.getClientIP(req) + ip := bc.getClientIP(req) if ip != tc.expectedIP { t.Errorf("expected ip %s, got %s", tc.expectedIP, ip) } @@ -484,7 +390,6 @@ func TestServeHTTP(t *testing.T) { config := CreateConfig() tests := []struct { name string - rateLimit uint expectedStatus uint challengePage string expectedBody string @@ -492,7 +397,6 @@ func TestServeHTTP(t *testing.T) { }{ { name: "Redirect to 302", - rateLimit: 0, challengePage: "/challenge", expectedStatus: http.StatusFound, expectedBody: "/challenge?destination=%2Fsomepath", @@ -500,7 +404,6 @@ func TestServeHTTP(t *testing.T) { }, { name: "403 when changing default challenge code", - rateLimit: 0, challengePage: "/challenge", expectedStatus: http.StatusFound, challengeCode: http.StatusForbidden, @@ -508,7 +411,6 @@ func TestServeHTTP(t *testing.T) { }, { name: "429 when challenging on same page", - rateLimit: 0, challengePage: "", expectedStatus: http.StatusTooManyRequests, expectedBody: "One moment while we verify your network connection", @@ -516,7 +418,6 @@ func TestServeHTTP(t *testing.T) { }, { name: "403 when challenging on same page", - rateLimit: 0, challengePage: "", expectedStatus: http.StatusForbidden, challengeCode: http.StatusForbidden, @@ -527,7 +428,6 @@ func TestServeHTTP(t *testing.T) { t.Run(tc.name, func(t *testing.T) { config.SiteKey = "test-site-key" config.SecretKey = "test-secret-key" - config.RateLimit = tc.rateLimit config.CaptchaProvider = "turnstile" config.ProtectRoutes = []string{"/"} config.ChallengeURL = tc.challengePage @@ -576,7 +476,6 @@ func TestServeHTTPAllowsGoodBotOnFirstRequest(t *testing.T) { config := CreateConfig() config.SiteKey = "test-site-key" config.SecretKey = "test-secret-key" - config.RateLimit = 0 config.ProtectRoutes = []string{"/"} config.GoodBots = []string{"yandex.com"} @@ -698,26 +597,6 @@ func TestNewCaptchaProtectValidation(t *testing.T) { modifyConfig: func(c *Config) { c.Mode = "invalid" }, expectError: "unknown mode", }, - { - name: "Invalid IPv4 mask - too small", - modifyConfig: func(c *Config) { c.IPv4SubnetMask = 5 }, - expectError: "invalid ipv4 mask", - }, - { - name: "Invalid IPv4 mask - too large", - modifyConfig: func(c *Config) { c.IPv4SubnetMask = 33 }, - expectError: "invalid ipv4 mask", - }, - { - name: "Invalid IPv6 mask - too small", - modifyConfig: func(c *Config) { c.IPv6SubnetMask = 5 }, - expectError: "invalid ipv6 mask", - }, - { - name: "Invalid IPv6 mask - too large", - modifyConfig: func(c *Config) { c.IPv6SubnetMask = 200 }, - expectError: "invalid ipv6 mask", - }, { name: "Invalid CIDR in ExemptIPs", modifyConfig: func(c *Config) { @@ -766,43 +645,6 @@ func TestRedactedConfigDoesNotExposeSecretKey(t *testing.T) { } } -func TestRateLimiting(t *testing.T) { - config := CreateConfig() - config.SiteKey = "test" - config.SecretKey = "test" - config.ProtectRoutes = []string{"/"} - config.RateLimit = 5 - config.Window = 10 - - bc, err := NewCaptchaProtect(context.Background(), nil, config, "test") - if err != nil { - t.Fatal(err) - } - - subnet := "192.168.0.0" - - // Register 5 requests (at rate limit) - for i := 0; i < 5; i++ { - bc.registerRequest(subnet) - if bc.trippedRateLimit(subnet) { - t.Errorf("Should not trip rate limit at %d requests", i+1) - } - } - - // 6th request should trip - bc.registerRequest(subnet) - if !bc.trippedRateLimit(subnet) { - t.Error("Should trip rate limit after exceeding") - } - - // Different subnet should not be affected - differentSubnet := "10.0.0.0" - bc.registerRequest(differentSubnet) - if bc.trippedRateLimit(differentSubnet) { - t.Error("Different subnet should not be rate limited") - } -} - func TestIsGoodBotWithParameters(t *testing.T) { config := CreateConfig() config.SiteKey = "test" @@ -842,7 +684,6 @@ func TestVerifiedCacheBypasses(t *testing.T) { config.SiteKey = "test" config.SecretKey = "test" config.ProtectRoutes = []string{"/"} - config.RateLimit = 0 // Always challenge unless verified bc, _ := NewCaptchaProtect(context.Background(), nil, config, "test") @@ -870,7 +711,6 @@ func TestShouldApplyRegexExcludeRoutesIgnoreQueryString(t *testing.T) { config.Mode = "regex" config.ProtectRoutes = []string{"^/"} config.ExcludeRoutes = []string{`\/oai\/request`, `\/node\/\d+\/(book-)?manifest`} - config.RateLimit = 0 bc, err := NewCaptchaProtect(context.Background(), nil, config, "test") if err != nil { @@ -918,8 +758,6 @@ func TestStatsPage(t *testing.T) { bc, _ := NewCaptchaProtect(context.Background(), nil, config, "test") - // Add some test data - bc.rateCache.Set("192.168.0.0", uint(10), 1*time.Hour) bc.verifiedCache.Set("1.2.3.4", true, 1*time.Hour) tests := []struct { @@ -948,10 +786,6 @@ func TestStatsPage(t *testing.T) { if err := json.Unmarshal(rr.Body.Bytes(), &stats); err != nil { t.Errorf("Failed to parse JSON: %v", err) } - // Check that we have expected keys - if _, ok := stats["rate"]; !ok { - t.Error("Stats JSON missing 'rate' key") - } if _, ok := stats["verified"]; !ok { t.Error("Stats JSON missing 'verified' key") } @@ -966,7 +800,6 @@ func TestProtectHttpMethods(t *testing.T) { config.SecretKey = "test" config.ProtectRoutes = []string{"/"} config.ProtectHttpMethods = []string{"GET", "POST"} - config.RateLimit = 0 // Always challenge bc, _ := NewCaptchaProtect(context.Background(), nil, config, "test") @@ -1035,17 +868,10 @@ func TestStatePersistence(t *testing.T) { config.SecretKey = "test" config.ProtectRoutes = []string{"/"} - // Manually save state by writing the file directly - // This tests the state format without relying on the background goroutine - // Use the new CacheEntry format with expiration timestamps + // Manually save state by writing the file directly. + // This tests restart persistence without relying on the background goroutine. futureExpiration := time.Now().Add(1 * time.Hour).UnixNano() jsonData, _ := json.Marshal(map[string]interface{}{ - "rate": map[string]map[string]interface{}{ - "192.168.0.0": { - "value": uint(10), - "expiration": float64(futureExpiration), - }, - }, "verified": map[string]map[string]interface{}{ "1.2.3.4": { "value": true, @@ -1062,41 +888,14 @@ func TestStatePersistence(t *testing.T) { bc2, _ := NewCaptchaProtect(context.Background(), nil, config, "test") bc2.loadStateFrom(tmpFile) - // Check rate cache - val, found := bc2.rateCache.Get("192.168.0.0") - if !found || val.(uint) != 10 { - t.Error("Rate cache state not persisted correctly") - } - // Check verified cache - _, found = bc2.verifiedCache.Get("1.2.3.4") + _, found := bc2.verifiedCache.Get("1.2.3.4") if !found { t.Error("Verified cache state not persisted correctly") } } -func TestRegisterRequestStopsIncrementingAfterRateLimitTrips(t *testing.T) { - tmpFile := filepath.Join(t.TempDir(), "state.json") - - bc := newStateOnlyCaptchaProtect(tmpFile, 2) - - for i := 0; i < 5; i++ { - bc.registerRequest("192.168.0.0") - } - - v, ok := bc.rateCache.Get("192.168.0.0") - if !ok { - t.Fatal("Expected rate cache entry") - } - if got, want := v.(uint), bc.config.RateLimit+1; got != want { - t.Fatalf("Expected sequential requests to stop incrementing at %d, got %d", want, got) - } - if got, want := bc.currentStateDirty(), uint64(bc.config.RateLimit+1); got != want { - t.Fatalf("Expected dirty counter to track only effective mutations, got %d want %d", got, want) - } -} - func TestStatePersistenceDisabledWithoutStateFile(t *testing.T) { config := CreateConfig() config.SiteKey = "test" @@ -1108,7 +907,6 @@ func TestStatePersistenceDisabledWithoutStateFile(t *testing.T) { t.Fatalf("NewCaptchaProtect failed: %v", err) } - bc.registerRequest("192.168.0.0") bc.markStateDirty() if bc.currentStateDirty() != 0 { @@ -1119,23 +917,11 @@ func TestStatePersistenceDisabledWithoutStateFile(t *testing.T) { } } -func TestStateSaveIntervalUsesFastCadenceOnlyWithReconciliation(t *testing.T) { - config := CreateConfig() - if got := stateSaveInterval(config); got != 60*time.Second { - t.Fatalf("default state save interval = %s, want 60s", got) - } - - config.EnableStateReconciliation = "true" - if got := stateSaveInterval(config); got != 10*time.Second { - t.Fatalf("reconciliation state save interval = %s, want 10s", got) - } -} - func TestSaveStateFlushesDirtyStateOnCanceledContext(t *testing.T) { tmpFile := filepath.Join(t.TempDir(), "state.json") - bc := newStateOnlyCaptchaProtect(tmpFile, 2) + bc := newStateOnlyCaptchaProtect(tmpFile) - bc.rateCache.Set("192.168.0.0", uint(1), time.Hour) + bc.verifiedCache.Set("192.168.0.10", true, time.Hour) bc.markStateDirty() ctx, cancel := context.WithCancel(context.Background()) @@ -1153,20 +939,20 @@ func TestSaveStateFlushesDirtyStateOnCanceledContext(t *testing.T) { } var saved struct { - Rate map[string]json.RawMessage `json:"rate"` + Verified map[string]json.RawMessage `json:"verified"` } if err := json.Unmarshal(data, &saved); err != nil { t.Fatalf("state file did not contain valid JSON: %v", err) } - if _, ok := saved.Rate["192.168.0.0"]; !ok { - t.Fatal("expected dirty rate entry to be persisted") + if _, ok := saved.Verified["192.168.0.10"]; !ok { + t.Fatal("expected dirty verified entry to be persisted") } } func TestSaveStateNowReturnsFalseAndKeepsDirtyStateOnWriteError(t *testing.T) { statePath := t.TempDir() - bc := newStateOnlyCaptchaProtect(statePath, 2) - bc.rateCache.Set("192.168.0.0", uint(1), time.Hour) + bc := newStateOnlyCaptchaProtect(statePath) + bc.verifiedCache.Set("192.168.0.10", true, time.Hour) bc.markStateDirty() if bc.saveStateNow() { @@ -1177,9 +963,9 @@ func TestSaveStateNowReturnsFalseAndKeepsDirtyStateOnWriteError(t *testing.T) { } } -func TestStateBookkeepingErrorBranches(t *testing.T) { +func TestStateBookkeepingCounters(t *testing.T) { missingFile := filepath.Join(t.TempDir(), "missing", "state.json") - bc := newStateOnlyCaptchaProtect(missingFile, 2) + bc := newStateOnlyCaptchaProtect(missingFile) bc.stateMu.Lock() bc.stateDirty = 1 @@ -1188,26 +974,6 @@ func TestStateBookkeepingErrorBranches(t *testing.T) { if got := bc.unsavedStateChanges(); got != 0 { t.Fatalf("unsavedStateChanges with saved counter ahead = %d, want 0", got) } - - bc.refreshStateFileModTime() - if !bc.stateFileModTime.IsZero() { - t.Fatal("refreshStateFileModTime should ignore missing state file") - } - - bc.reconcileStateFromFileIfChanged() - if !bc.stateFileModTime.IsZero() { - t.Fatal("reconcileStateFromFileIfChanged should ignore missing state file") - } - - invalidFile := filepath.Join(t.TempDir(), "state.json") - if err := os.WriteFile(invalidFile, []byte("{invalid json"), 0600); err != nil { - t.Fatalf("failed to write invalid state file: %v", err) - } - invalidBC := newStateOnlyCaptchaProtect(invalidFile, 2) - invalidBC.reconcileStateFromFileIfChanged() - if !invalidBC.stateFileModTime.IsZero() { - t.Fatal("failed reconciliation should not advance state file mod time") - } } func TestVerifyChallengePage(t *testing.T) { @@ -1643,9 +1409,8 @@ func TestLoadStateInvalidJSON(t *testing.T) { } bc.loadStateFrom(tmpFile) - // Caches should be empty - if bc.rateCache.ItemCount() != 0 { - t.Error("Rate cache should be empty after failed load") + if bc.verifiedCache.ItemCount() != 0 { + t.Error("Verified cache should be empty after failed load") } // Clean up the file before temp dir cleanup