Fix access check form: use GET so results render

The access check form POSTed and re-rendered :new with a 200 HTML response, which Turbo rejects ("Form responses must redirect to another location"), so the result panel never appeared. Since the check is a read-only query, switch to a GET form and fold the lookup into the new action. Results are now bookmarkable via the URL. Bump version to 0.16.2. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fix invalid require-trusted-types-for CSP directive
2026-06-21 15:42:57 +10:00 · 2026-06-21 15:39:35 +10:00 · 2026-06-21 14:02:29 +10:00 · 2026-06-21 13:57:26 +10:00 · 2026-06-21 13:51:28 +10:00 · 2026-06-21 13:51:23 +10:00
23 changed files with 348 additions and 212 deletions
--- a/.github/workflows/build.yml
+++ b/.github/workflows/build.yml
@@ -0,0 +1,56 @@
+name: Build and publish image
+
+on:
+  push:
+    branches: [ main ]
+    tags: [ 'v*' ]
+
+# Only one build per ref at a time; cancel superseded main builds.
+concurrency:
+  group: build-${{ github.ref }}
+  cancel-in-progress: ${{ github.ref == 'refs/heads/main' }}
+
+jobs:
+  build:
+    runs-on: ubuntu-latest
+    permissions:
+      contents: read
+      packages: write  # Required to push to GHCR
+
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v5
+
+      - name: Set up Docker Buildx
+        uses: docker/setup-buildx-action@v3
+
+      - name: Log in to GitHub Container Registry
+        uses: docker/login-action@v3
+        with:
+          registry: ghcr.io
+          username: ${{ github.actor }}
+          password: ${{ secrets.GITHUB_TOKEN }}
+
+      - name: Extract image metadata (tags, labels)
+        id: meta
+        uses: docker/metadata-action@v5
+        with:
+          images: ghcr.io/${{ github.repository }}
+          tags: |
+            type=edge,branch=main
+            type=sha,prefix=sha-,format=short,enable={{is_default_branch}}
+            type=semver,pattern=v{{version}}
+            type=semver,pattern=v{{major}}.{{minor}}
+          flavor: |
+            latest=auto
+
+      - name: Build and push
+        uses: docker/build-push-action@v6
+        with:
+          context: .
+          platforms: linux/amd64
+          push: true
+          tags: ${{ steps.meta.outputs.tags }}
+          labels: ${{ steps.meta.outputs.labels }}
+          cache-from: type=gha
+          cache-to: type=gha,mode=max
--- a/.ruby-version
+++ b/.ruby-version
@@ -1 +1 @@
-4.0.3
+4.0.5
--- a/2
+++ b/2
@@ -8,7 +8,7 @@
 # For a containerized dev environment, see Dev Containers: https://guides.rubyonrails.org/getting_started_with_devcontainer.html

 # Make sure RUBY_VERSION matches the Ruby version in .ruby-version
-ARG RUBY_VERSION=4.0.3
+ARG RUBY_VERSION=4.0.5
 FROM docker.io/library/ruby:$RUBY_VERSION-slim AS base

 LABEL org.opencontainers.image.source=https://github.com/dkam/clinch
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -1,7 +1,7 @@
 GEM
  remote: https://rubygems.org/
  specs:
-    action_text-trix (2.1.18)
+    action_text-trix (2.1.19)
      railties
    actioncable (8.1.3)
      actionpack (= 8.1.3)
@@ -85,9 +85,9 @@ GEM
    bigdecimal (4.1.2)
    bindata (2.5.1)
    bindex (0.8.1)
-    bootsnap (1.24.1)
+    bootsnap (1.24.6)
      msgpack (~> 1.2)
-    brakeman (8.0.4)
+    brakeman (8.0.5)
      racc
    builder (3.3.0)
    bundler-audit (0.9.3)
@@ -102,11 +102,11 @@ GEM
      rack-test (>= 0.6.3)
      regexp_parser (>= 1.5, < 3.0)
      xpath (~> 3.2)
-    cbor (0.5.10.2)
+    cbor (0.5.10.3)
    childprocess (5.1.0)
      logger (~> 1.5)
    chunky_png (1.4.0)
-    concurrent-ruby (1.3.6)
+    concurrent-ruby (1.3.7)
    connection_pool (3.0.2)
    cose (1.3.1)
      cbor (~> 0.5.9)
@@ -131,12 +131,12 @@ GEM
    ffi (1.17.4-arm64-darwin)
    ffi (1.17.4-x86_64-linux-gnu)
    ffi (1.17.4-x86_64-linux-musl)
-    fugit (1.12.1)
+    fugit (1.12.2)
      et-orbi (~> 1.4)
      raabro (~> 1.4)
    globalid (1.3.0)
      activesupport (>= 6.1)
-    i18n (1.14.8)
+    i18n (1.15.2)
      concurrent-ruby (~> 1.0)
    image_processing (1.14.0)
      mini_magick (>= 4.9.5, < 6)
@@ -151,13 +151,13 @@ GEM
      prism (>= 1.3.0)
      rdoc (>= 4.0.0)
      reline (>= 0.4.2)
-    jbuilder (2.14.1)
+    jbuilder (2.15.1)
      actionview (>= 7.0.0)
      activesupport (>= 7.0.0)
-    json (2.19.4)
-    jwt (3.1.2)
+    json (2.19.9)
+    jwt (3.2.0)
      base64
-    kamal (2.11.0)
+    kamal (2.12.0)
      activesupport (>= 7.0)
      base64 (~> 0.2)
      bcrypt_pbkdf (~> 1.0)
@@ -186,14 +186,14 @@ GEM
      net-imap
      net-pop
      net-smtp
-    marcel (1.1.0)
+    marcel (1.2.1)
    matrix (0.4.3)
    mini_magick (5.3.1)
      logger
    mini_mime (1.1.5)
    minitest (5.27.0)
-    msgpack (1.8.0)
-    net-imap (0.6.4)
+    msgpack (1.8.3)
+    net-imap (0.6.4.1)
      date
      net-protocol
    net-pop (0.1.2)
@@ -208,25 +208,25 @@ GEM
      net-protocol
    net-ssh (7.3.2)
    nio4r (2.7.5)
-    nokogiri (1.19.3-aarch64-linux-gnu)
+    nokogiri (1.19.4-aarch64-linux-gnu)
      racc (~> 1.4)
-    nokogiri (1.19.3-aarch64-linux-musl)
+    nokogiri (1.19.4-aarch64-linux-musl)
      racc (~> 1.4)
-    nokogiri (1.19.3-arm-linux-gnu)
+    nokogiri (1.19.4-arm-linux-gnu)
      racc (~> 1.4)
-    nokogiri (1.19.3-arm-linux-musl)
+    nokogiri (1.19.4-arm-linux-musl)
      racc (~> 1.4)
-    nokogiri (1.19.3-arm64-darwin)
+    nokogiri (1.19.4-arm64-darwin)
      racc (~> 1.4)
-    nokogiri (1.19.3-x86_64-linux-gnu)
+    nokogiri (1.19.4-x86_64-linux-gnu)
      racc (~> 1.4)
-    nokogiri (1.19.3-x86_64-linux-musl)
+    nokogiri (1.19.4-x86_64-linux-musl)
      racc (~> 1.4)
-    openssl (4.0.1)
+    openssl (4.0.2)
    openssl-signature_algorithm (1.3.0)
      openssl (> 2.0)
    ostruct (0.6.3)
-    parallel (1.28.0)
+    parallel (2.1.0)
    parser (3.3.11.1)
      ast (~> 2.4.1)
      racc
@@ -238,11 +238,11 @@ GEM
      actionpack (>= 7.0.0)
      activesupport (>= 7.0.0)
      rack
-    psych (5.3.1)
+    psych (5.4.0)
      date
      stringio
    public_suffix (7.0.5)
-    puma (8.0.1)
+    puma (8.0.2)
      nio4r (~> 2.0)
    raabro (1.4.0)
    racc (1.8.1)
@@ -299,11 +299,11 @@ GEM
      chunky_png (~> 1.0)
      rqrcode_core (~> 2.0)
    rqrcode_core (2.1.0)
-    rubocop (1.84.2)
+    rubocop (1.87.0)
      json (~> 2.3)
      language_server-protocol (~> 3.17.0.2)
      lint_roller (~> 1.1.0)
-      parallel (~> 1.10)
+      parallel (>= 1.10)
      parser (>= 3.3.0.2)
      rainbow (>= 2.2.2, < 4.0)
      regexp_parser (>= 2.9.3, < 3.0)
@@ -321,20 +321,20 @@ GEM
    ruby-vips (2.3.0)
      ffi (~> 1.12)
      logger
-    rubyzip (3.2.2)
+    rubyzip (3.4.0)
    safety_net_attestation (0.5.0)
      jwt (>= 2.0, < 4.0)
    securerandom (0.4.1)
-    selenium-webdriver (4.43.0)
+    selenium-webdriver (4.45.0)
      base64 (~> 0.2)
      logger (~> 1.4)
      rexml (~> 3.2, >= 3.2.5)
      rubyzip (>= 1.2.2, < 4.0)
      websocket (~> 1.0)
-    sentry-rails (6.5.0)
+    sentry-rails (6.6.2)
      railties (>= 5.2.0)
-      sentry-ruby (~> 6.5.0)
-    sentry-ruby (6.5.0)
+      sentry-ruby (~> 6.6.2)
+    sentry-ruby (6.6.2)
      bigdecimal
      concurrent-ruby (~> 1.0, >= 1.0.2)
      logger
@@ -344,7 +344,7 @@ GEM
      simplecov_json_formatter (~> 0.1)
    simplecov-html (0.13.2)
    simplecov_json_formatter (0.1.4)
-    solid_cable (3.0.12)
+    solid_cable (4.0.0)
      actioncable (>= 7.2)
      activejob (>= 7.2)
      activerecord (>= 7.2)
@@ -360,13 +360,13 @@ GEM
      fugit (~> 1.11)
      railties (>= 7.1)
      thor (>= 1.3.1)
-    sqlite3 (2.9.3-aarch64-linux-gnu)
-    sqlite3 (2.9.3-aarch64-linux-musl)
-    sqlite3 (2.9.3-arm-linux-gnu)
-    sqlite3 (2.9.3-arm-linux-musl)
-    sqlite3 (2.9.3-arm64-darwin)
-    sqlite3 (2.9.3-x86_64-linux-gnu)
-    sqlite3 (2.9.3-x86_64-linux-musl)
+    sqlite3 (2.9.5-aarch64-linux-gnu)
+    sqlite3 (2.9.5-aarch64-linux-musl)
+    sqlite3 (2.9.5-arm-linux-gnu)
+    sqlite3 (2.9.5-arm-linux-musl)
+    sqlite3 (2.9.5-arm64-darwin)
+    sqlite3 (2.9.5-x86_64-linux-gnu)
+    sqlite3 (2.9.5-x86_64-linux-musl)
    sshkit (1.25.0)
      base64
      logger
@@ -374,10 +374,10 @@ GEM
      net-sftp (>= 2.1.2)
      net-ssh (>= 2.8.0)
      ostruct
-    standard (1.54.0)
+    standard (1.55.0)
      language_server-protocol (~> 3.17.0.2)
      lint_roller (~> 1.0)
-      rubocop (~> 1.84.0)
+      rubocop (~> 1.87.0)
      standard-custom (~> 1.0.0)
      standard-performance (~> 1.8)
    standard-custom (1.0.2)
@@ -389,20 +389,20 @@ GEM
    stimulus-rails (1.3.4)
      railties (>= 6.0.0)
    stringio (3.2.0)
-    tailwindcss-rails (4.4.0)
+    tailwindcss-rails (4.6.0)
      railties (>= 7.0.0)
      tailwindcss-ruby (~> 4.0)
-    tailwindcss-ruby (4.2.4)
-    tailwindcss-ruby (4.2.4-aarch64-linux-gnu)
-    tailwindcss-ruby (4.2.4-aarch64-linux-musl)
-    tailwindcss-ruby (4.2.4-arm64-darwin)
-    tailwindcss-ruby (4.2.4-x86_64-linux-gnu)
-    tailwindcss-ruby (4.2.4-x86_64-linux-musl)
+    tailwindcss-ruby (4.3.1)
+    tailwindcss-ruby (4.3.1-aarch64-linux-gnu)
+    tailwindcss-ruby (4.3.1-aarch64-linux-musl)
+    tailwindcss-ruby (4.3.1-arm64-darwin)
+    tailwindcss-ruby (4.3.1-x86_64-linux-gnu)
+    tailwindcss-ruby (4.3.1-x86_64-linux-musl)
    thor (1.5.0)
-    thruster (0.1.20)
-    thruster (0.1.20-aarch64-linux)
-    thruster (0.1.20-arm64-darwin)
-    thruster (0.1.20-x86_64-linux)
+    thruster (0.1.21)
+    thruster (0.1.21-aarch64-linux)
+    thruster (0.1.21-arm64-darwin)
+    thruster (0.1.21-x86_64-linux)
    timeout (0.6.1)
    tpm-key_attestation (0.14.1)
      bindata (~> 2.4)
@@ -432,13 +432,13 @@ GEM
      safety_net_attestation (~> 0.5.0)
      tpm-key_attestation (~> 0.14.0)
    websocket (1.2.11)
-    websocket-driver (0.8.0)
+    websocket-driver (0.8.1)
      base64
      websocket-extensions (>= 0.1.0)
    websocket-extensions (0.1.5)
    xpath (3.2.0)
      nokogiri (~> 1.8)
-    zeitwerk (2.7.5)
+    zeitwerk (2.8.2)

 PLATFORMS
  aarch64-linux
--- a/SECURITY_REVIEW_TODO.md
+++ b/SECURITY_REVIEW_TODO.md
@@ -0,0 +1,110 @@
+# Security Review — Tracking
+
+Status of findings from the multi-surface security review (OIDC/OAuth2, ForwardAuth,
+WebAuthn/TOTP, sessions, admin/config). Work landed on branch
+`security/forward-auth-and-consent-csrf`.
+
+## ✅ Done (branch `security/forward-auth-and-consent-csrf`)
+
+All HIGH findings are closed. Each fix has tests; suite is green.
+
+| Commit | Fix | Sev |
+|--------|-----|-----|
+| `703d24e` | ForwardAuth fail-open when no host header; consent endpoint CSRF | HIGH ×2 |
+| `8a095e4` | Bearer API-key skipped group check at use-time | HIGH |
+| `96a657e` | Open redirect via unvalidated `X-Forwarded-Host` in login redirect | HIGH |
+| `84ed462` | `CLINCH_HOST` made mandatory in deployed envs; dropped request-host fallback | MEDIUM |
+| `f38ac2e` | TOTP code replay within drift window (+ latent plaintext backup-code bug) | HIGH |
+| `406a79d` | SSRF via `backchannel_logout_uri` (metadata/loopback/RFC1918) | HIGH |
+| `57d7d1f` | Host-auth regex unanchored (`evil-example.com` matched) | HIGH |
+| `89bd5f1` | Disabled user could complete 2FA mid-flow / keep session; enforce active status | HIGH |
+| `cd862c7` | TOTP/backup/OAuth/PKCE `code` params not filtered from logs | MEDIUM |
+| `2426687` | `revoke_family!` didn't revoke access tokens on refresh-token reuse | HIGH |
+| `44892e3` | WebAuthn clone detection logged but didn't block; false-positive on synced passkeys | HIGH |
+| `d49e7ce` | CSP `unsafe-inline` removed (script-src + style-src → nonces) | HIGH |
+
+**Verified false positive (no change):** PKCE *is* required by default —
+`require_pkce` column defaults to `true` (`db/schema.rb`), token endpoint enforces
+it, admin UI exposes the opt-out. Operational check: confirm no legacy confidential
+apps sit on `require_pkce = false`.
+
+**Follow-up before relying on CSP change:** do one manual browser pass (DevTools
+console) on `/signin`, OAuth consent, a Turbo navigation, dark-mode toggle, and a
+WebAuthn sign-in — expect zero CSP violations. Dev is report-only so violations
+surface as warnings without breaking. Fallback if style-src surprises: keep
+`style-src 'unsafe-inline'`, ship script-src only.
+
+## ☐ Remaining — MEDIUM
+
+- [ ] **`id_token_hint` ignored at OIDC logout** — any client can redirect logout to
+      any other registered client's post-logout URI. Validate the hint's `aud` and
+      scope the redirect to that app. `app/controllers/oidc_controller.rb` (logout).
+- [ ] **`offline_access` doesn't gate refresh-token issuance** — refresh tokens are
+      minted unconditionally; gate on the granted scope.
+      `app/controllers/oidc_controller.rb` (authorization_code grant, ~line 564).
+- [ ] **CSP-report endpoint hardening** — unauthenticated, no rate limit / body-size
+      cap, logs raw CRLF (log injection). Sanitize values, cap size, rate-limit.
+      `app/controllers/api/csp_controller.rb`.
+- [ ] **Port not stripped from `X-Forwarded-Host`** in main verify + bearer paths →
+      403 outages on non-standard ports (also a correctness bug). Reuse the
+      port-stripping done in `check_forward_auth_token`.
+      `app/controllers/api/forward_auth_controller.rb`.
+- [ ] **WebAuthn `acr:"2"` without enforced user verification** — `user_verification:
+      "preferred"` lets a PIN-less key authenticate yet reports verified 2FA. Use
+      `"required"`, or downgrade `acr` to `"1"` when the UV flag is absent.
+      `app/controllers/sessions_controller.rb` (webauthn_challenge/verify),
+      `app/controllers/webauthn_controller.rb`.
+- [ ] **`RESERVED_CLAIMS` incomplete** — missing `at_hash`/`auth_time`/`acr`; and
+      `ApplicationUserClaims` has no reserved-name validation (User/Group do). Could
+      let a custom claim overwrite a security claim. `app/services/oidc_jwt_service.rb`,
+      `app/models/application_user_claim.rb`.
+- [ ] **`reset_session` not called on login** — defensive best practice for an IdP;
+      clears pre-auth session state. `app/controllers/concerns/authentication.rb`
+      (`start_new_session_for`).
+- [x] **Hardcoded private IP `192.168.2.246`** in `config/environments/production.rb`
+      — removed; it was redundant with the `192.168.0.0/16` regex already in the
+      `CLINCH_ALLOW_INTERNAL_IPS` block.
+- [ ] **CSP `form-action` widened by unvalidated `redirect_uri`** before auth — only
+      add to `form-action` if the client_id+redirect_uri is a registered pair.
+      `app/controllers/concerns/authentication.rb` (`allow_oauth_redirect_in_csp`).
+- [ ] **SVG `style` attribute permits `url()`/`expression()`** — mitigated today by
+      `Content-Disposition: attachment`, but fragile. Sanitize CSS values or drop
+      `style` from the allowlist. `app/models/svg_scrubber.rb`.
+- [ ] **WebAuthn error messages leak internals** — return generic errors to client,
+      log detail server-side. `app/controllers/sessions_controller.rb`,
+      `app/controllers/webauthn_controller.rb`.
+- [ ] **Account enumeration via webauthn challenge** — distinguishes "user not found"
+      vs "no passkey". Return a uniform message. `app/controllers/sessions_controller.rb`
+      (`webauthn_challenge`).
+- [ ] **`token_family_id` only 31 bits** (`SecureRandom.random_number(2**31)`) —
+      birthday collision ~46k; use a UUID/string. `app/models/oidc_refresh_token.rb`.
+- [ ] **Session cookie uses sequential integer DB id** — HMAC-signed so not forgeable,
+      but consider a random `token` column (Rails 8 generator default).
+      `app/models/session.rb`, `app/controllers/concerns/authentication.rb`.
+- [ ] **Login rate-limit is IP-only** — no account lockout (distributed brute force /
+      credential stuffing). Add failed-count + `locked_until` on users.
+- [ ] **Backup-code rate limit not reset on success** and is cache-based (resets on
+      cache flush). Reset on success; consider DB-backed counter. `app/models/user.rb`.
+
+## ☐ Remaining — LOW / INFO
+
+- [ ] Public clients can't revoke their own tokens (revoke endpoint requires secret).
+- [ ] Basic-auth client creds not URL-decoded per RFC 6749 §2.3.1.
+- [ ] `token_hmac` columns nullable at DB level despite model `presence: true`.
+- [ ] Group names allow commas → injection into `X-Remote-Groups` (false memberships
+      downstream). Add a format validator. `app/models/group.rb`.
+- [ ] `fa_token` leaks in redirect URL / Referer / history (60s TTL, host-bound).
+- [ ] Admin `domain_pattern` allows ReDoS — add a format validator.
+      `app/models/application.rb`.
+- [ ] Forced-TOTP-setup login path can redirect-loop (`totp_required` + no TOTP).
+- [ ] `complete_setup` creates an unprompted session for any authenticated user.
+- [ ] Password min length only 8 — consider 12 + a max (bcrypt 72-byte truncation).
+- [ ] `support_unencrypted_data: true` left enabled (TOTP secret encryption migration).
+      `config/initializers/active_record_encryption.rb`.
+- [ ] All crypto keys derived from a single `SECRET_KEY_BASE` root — document setting
+      independent `ACTIVE_RECORD_ENCRYPTION_*` keys in production.
+- [ ] Log injection via user `email_address` in ForwardAuth logs (strip CRLF / use
+      structured logging). `app/controllers/api/forward_auth_controller.rb`.
+- [ ] WebAuthn RP ID is the registrable domain (cross-subdomain credential roaming) —
+      set `CLINCH_RP_ID` to the exact host unless roaming is intended.
+      `config/initializers/webauthn.rb`.
--- a/app/controllers/admin/access_checks_controller.rb
+++ b/app/controllers/admin/access_checks_controller.rb
@@ -2,17 +2,12 @@ module Admin
  class AccessChecksController < BaseController
    def new
      load_options
-    end
-
-    def create
-      load_options
      @user = User.find_by(id: params[:user_id])
      @application = Application.find_by(id: params[:application_id])
-      return render :new unless @user && @application
+      return unless @user && @application

      @allowed = @application.user_allowed?(@user)
      @via = @user.groups & @application.allowed_groups
-      render :new
    end

    private
--- a/app/controllers/concerns/authentication.rb
+++ b/app/controllers/concerns/authentication.rb
@@ -62,9 +62,14 @@ module Authentication
    return if redirect_host.blank?

    csp = request.content_security_policy
-    return unless csp&.respond_to?(:form_action) && csp.form_action.respond_to?(:<<)
+    return unless csp

-    csp.form_action << "https://#{redirect_host}"
+    # NOTE: `csp.form_action` (no args) is destructive — it deletes the directive
+    # and returns its old value, so reading it twice yields nil. Mutate the
+    # underlying `directives` hash (a public reader of the real values) instead.
+    form_action = (csp.directives["form-action"] ||= ["'self'"])
+    host = "https://#{redirect_host}"
+    form_action << host unless form_action.include?(host)
  rescue URI::InvalidURIError
    nil
  end
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -31,7 +31,7 @@ module ApplicationHelper
    end
    lines << "OIDC_DISCOVERY_URL=#{OidcJwtService.issuer_url}"
    lines << "OIDC_PROVIDER_NAME='Clinch'"
-    lines << "OIDC_REQUIRE_PKCE=#{application.requires_pkce? ? 'true' : 'false'}"
+    lines << "OIDC_REQUIRE_PKCE=#{application.requires_pkce? ? "true" : "false"}"
    lines
  end

--- a/app/lib/private_address_check.rb
+++ b/app/lib/private_address_check.rb
@@ -35,7 +35,7 @@ module PrivateAddressCheck
    return [ip] if ip

    Resolv.getaddresses(host.to_s).filter_map { |a| parse_ip(a) }
-  rescue StandardError
+  rescue
    # Resolution failure: surface no addresses. Callers treat "can't resolve" as
    # not-provably-internal; the dial itself will then fail safely.
    []
--- a/app/views/admin/access_checks/new.html.erb
+++ b/app/views/admin/access_checks/new.html.erb
@@ -5,7 +5,7 @@

 <div class="bg-white dark:bg-gray-800 shadow sm:rounded-lg">
  <div class="px-4 py-5 sm:p-6">
-    <%= form_with url: admin_access_path, method: :post, class: "space-y-4" do |form| %>
+    <%= form_with url: admin_access_path, method: :get, class: "space-y-4" do |form| %>
      <div class="grid grid-cols-1 gap-4 sm:grid-cols-2">
        <div>
          <%= form.label :user_id, "User", class: "block text-sm font-medium text-gray-700 dark:text-gray-300" %>
--- a/config/environments/production.rb
+++ b/config/environments/production.rb
@@ -139,9 +139,6 @@ Rails.application.configure do

  # Allow internal IP access for cross-compose or host networking
  if ENV["CLINCH_ALLOW_INTERNAL_IPS"] == "true"
-    # Specific host IP
-    allowed_hosts << "192.168.2.246"
-
    # Private IP ranges for internal network access
    allowed_hosts += [
      /192\.168\.\d+\.\d+/,    # 192.168.0.0/16 private network
@@ -160,17 +157,5 @@ Rails.application.configure do
  # Skip DNS rebinding protection for the default health check endpoint.
  config.host_authorization = {exclude: ->(request) { request.path == "/up" }}

-  # Sentry configuration for production
-  # Only enabled if SENTRY_DSN environment variable is set
-  if ENV["SENTRY_DSN"].present?
-    config.sentry.enabled = true
-
-    # Performance monitoring: sample 20% of transactions for traces
-    # Adjust based on your traffic volume and Sentry plan limits
-    config.sentry.traces_sample_rate = ENV.fetch("SENTRY_TRACES_SAMPLE_RATE", 0.2).to_f
-
-    # Continuous profiling: disabled by default in production due to cost
-    # Enable temporarily for performance investigations if needed
-    config.sentry.profiles_sample_rate = ENV.fetch("SENTRY_PROFILES_SAMPLE_RATE", 0.0).to_f
-  end
+  # Sentry is configured in config/initializers/sentry.rb, gated on SENTRY_DSN.
 end
--- a/config/initializers/content_security_policy.rb
+++ b/config/initializers/content_security_policy.rb
@@ -53,9 +53,10 @@ Rails.application.configure do
    # Child sources: Allow self for any future iframes
    policy.child_src :self

-    # Additional security headers for WebAuthn
-    # Required for WebAuthn to work properly
-    policy.require_trusted_types_for :none
+    # Do not enforce Trusted Types. The only valid value for
+    # require-trusted-types-for is 'script'; there is no 'none' token, so
+    # emitting it produces an invalid directive that browsers reject. To leave
+    # Trusted Types unenforced (needed for WebAuthn), omit the directive entirely.

    # CSP reporting using report_uri (supported method)
    policy.report_uri "/api/csp-violation-report"
--- a/config/initializers/sentry.rb
+++ b/config/initializers/sentry.rb
@@ -1,62 +1,44 @@
-# Sentry configuration for error tracking and performance monitoring
-# Only initializes if SENTRY_DSN environment variable is set
+# Sentry configuration for error tracking and performance monitoring.
+# Only initializes if the SENTRY_DSN environment variable is set.

 return unless ENV["SENTRY_DSN"].present?

-Rails.application.configure do
-  config.sentry.dsn = ENV["SENTRY_DSN"]
+Sentry.init do |config|
+  config.dsn = ENV["SENTRY_DSN"]

-  # Set environment (defaults to Rails.env)
-  config.sentry.environment = ENV["SENTRY_ENVIRONMENT"] || Rails.env
+  # Environment label (defaults to Rails.env)
+  config.environment = ENV["SENTRY_ENVIRONMENT"] || Rails.env

-  # Set release version from Git or environment variable
-  config.sentry.release = ENV["SENTRY_RELEASE"] || `git rev-parse HEAD 2>/dev/null`.strip.presence || nil
+  # Release version from an env var or the current Git SHA
+  config.release = ENV["SENTRY_RELEASE"] || `git rev-parse HEAD 2>/dev/null`.strip.presence

-  # Sample rate for performance monitoring (0.0 to 1.0)
-  config.sentry.traces_sample_rate = ENV.fetch("SENTRY_TRACES_SAMPLE_RATE", 0.1).to_f
+  # Only report from production unless explicitly enabled elsewhere.
+  config.enabled_environments =
+    if ENV["SENTRY_ENABLED_IN_DEVELOPMENT"] == "true"
+      %w[production development]
+    else
+      %w[production]
+    end

-  # Enable profiling in development/staging, disable in production unless explicitly enabled
-  config.sentry.profiles_sample_rate = if Rails.env.production?
+  # Don't send cookies, request bodies, or user IPs by default.
+  config.send_default_pii = false
+
+  # Breadcrumbs for debugging
+  config.breadcrumbs_logger = [:active_support_logger, :http_logger]
+
+  # Performance monitoring sample rate (0.0 to 1.0)
+  config.traces_sample_rate = ENV.fetch("SENTRY_TRACES_SAMPLE_RATE", 0.1).to_f
+
+  # Profiling: disabled in production by default due to cost.
+  config.profiles_sample_rate =
+    if Rails.env.production?
      ENV.fetch("SENTRY_PROFILES_SAMPLE_RATE", 0.0).to_f
    else
      ENV.fetch("SENTRY_PROFILES_SAMPLE_RATE", 0.5).to_f
    end

-  # Include additional context
-  config.sentry.before_send = lambda do |event, hint|
-    # Filter out sensitive information
-    if event.context[:extra]
-      event.context[:extra].reject! { |key, value|
-        key.to_s.match?(/password|secret|token|key/i) || value.to_s.match?(/password|secret/i)
-      }
-    end
-
-    # Filter sensitive parameters
-    if event.context[:request]
-      event.context[:request].reject! { |key, value|
-        key.to_s.match?(/password|secret|token|key|authorization/i)
-      }
-    end
-
-    event
-  end
-
-  # Include breadcrumbs for debugging
-  config.sentry.breadcrumbs_logger = [:active_support_logger, :http_logger]
-
-  # Send session data for user context
-  config.sentry.user_context = lambda do
-    if Current.user.present?
-      {
-        id: Current.user.id,
-        email: Current.user.email_address,
-        admin: Current.user.admin?
-      }
-    end
-  end
-
  # Ignore common non-critical exceptions
-  config.sentry.excluded_exceptions += [
+  config.excluded_exceptions += [
    "ActionController::RoutingError",
    "ActionController::InvalidAuthenticityToken",
    "ActionController::UnknownFormat",
@@ -66,75 +48,38 @@ Rails.application.configure do
    "ActiveRecord::RecordNotFound"
  ]

-  # Add CSP-specific tags for security events
-  config.sentry.tags = lambda do
-    {
-      # Add application context
+  # Attach application/user context and scrub anything sensitive before sending.
+  config.before_send = lambda do |event, _hint|
+    event.tags = (event.tags || {}).merge(
      app_name: "clinch",
-      app_environment: Rails.env,
-      # Add CSP policy status
-      csp_enabled: defined?(Rails.application.config.content_security_policy) &&
-        Rails.application.config.content_security_policy.present?
-    }
+      app_environment: Rails.env
+    )
+
+    if defined?(Current) && Current.respond_to?(:user) && Current.user
+      event.user = (event.user || {}).merge(
+        id: Current.user.id,
+        email: Current.user.email_address,
+        admin: Current.user.admin?
+      )
    end

-  # Enhance before_send to handle CSP events properly
-  config.sentry.before_send = lambda do |event, hint|
-    # Filter out sensitive information
-    if event.context[:extra]
-      event.context[:extra].reject! { |key, value|
+    if event.extra.is_a?(Hash)
+      event.extra.reject! do |key, value|
        key.to_s.match?(/password|secret|token|key/i) || value.to_s.match?(/password|secret/i)
-      }
      end
-
-    # Filter sensitive parameters
-    if event.context[:request]
-      event.context[:request].reject! { |key, value|
-        key.to_s.match?(/password|secret|token|key|authorization/i)
-      }
-    end
-
-    # Special handling for CSP violations
-    if event.tags&.dig(:csp_violation)
-      # Ensure CSP violations have proper security context
-      event.context[:server] = event.context[:server] || {}
-      event.context[:server][:name] = "clinch-auth-service"
-      event.context[:server][:environment] = Rails.env
-
-      # Add additional security context
-      event.context[:extra] ||= {}
-      event.context[:extra][:security_context] = {
-        csp_reporting: true,
-        user_authenticated: event.context[:user].present?,
-        request_origin: event.context[:request]&.dig(:headers, "Origin"),
-        request_referer: event.context[:request]&.dig(:headers, "Referer")
-      }
    end

    event
  end

-  # Add CSP-specific breadcrumbs for security events
-  config.sentry.before_breadcrumb = lambda do |breadcrumb, hint|
-    # Filter out sensitive breadcrumb data
-    if breadcrumb[:data]
-      breadcrumb[:data].reject! { |key, value|
-        key.to_s.match?(/password|secret|token|key|authorization/i) ||
-          value.to_s.match?(/password|secret/i)
-      }
+  # Scrub sensitive data out of breadcrumbs.
+  config.before_breadcrumb = lambda do |breadcrumb, _hint|
+    if breadcrumb.data.is_a?(Hash)
+      breadcrumb.data.reject! do |key, value|
+        key.to_s.match?(/password|secret|token|key|authorization/i) || value.to_s.match?(/password|secret/i)
      end
-
-    # Mark CSP-related events
-    if breadcrumb[:message]&.include?("CSP Violation") ||
-        breadcrumb[:category]&.include?("csp")
-      breadcrumb[:data] ||= {}
-      breadcrumb[:data][:security_event] = true
-      breadcrumb[:data][:csp_violation] = true
    end

    breadcrumb
  end
-
-  # Only send errors in production unless explicitly enabled
-  config.sentry.enabled = Rails.env.production? || ENV["SENTRY_ENABLED_IN_DEVELOPMENT"] == "true"
 end
--- a/config/initializers/version.rb
+++ b/config/initializers/version.rb
@@ -1,5 +1,5 @@
 # frozen_string_literal: true

 module Clinch
-  VERSION = "0.16.0"
+  VERSION = "0.16.2"
 end
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -96,7 +96,6 @@ Rails.application.routes.draw do
    end
    resources :groups
    get "access", to: "access_checks#new"
-    post "access", to: "access_checks#create"
  end

  # Render dynamic PWA files from app/views/pwa/* (remember to link manifest in application.html.erb)
--- a/test/controllers/admin/access_checks_controller_test.rb
+++ b/test/controllers/admin/access_checks_controller_test.rb
@@ -15,8 +15,8 @@ module Admin
      assert_match "alice@example.com", response.body
    end

-    test "create returns 'can access' with via group when user is in an allowed group" do
-      post admin_access_path, params: {
+    test "returns 'can access' with via group when user is in an allowed group" do
+      get admin_access_path, params: {
        user_id: users(:alice).id,
        application_id: @kavita.id
      }
@@ -25,9 +25,9 @@ module Admin
      assert_match "Administrators", response.body # alice is in admin_group; kavita has admin_group
    end

-    test "create returns 'cannot access' with reason when user shares no group with the app" do
+    test "returns 'cannot access' with reason when user shares no group with the app" do
      lonely = User.create!(email_address: "lonely@example.com", password: "password123", skip_auto_assign: true)
-      post admin_access_path, params: {
+      get admin_access_path, params: {
        user_id: lonely.id,
        application_id: @kavita.id
      }
@@ -36,8 +36,8 @@ module Admin
      assert_match "shares no group", response.body
    end

-    test "create renders form unchanged when ids are missing" do
-      post admin_access_path, params: {user_id: "", application_id: ""}
+    test "renders form unchanged when ids are missing" do
+      get admin_access_path, params: {user_id: "", application_id: ""}
      assert_response :success
      # No result panel should render. The panel-only phrases:
      refute_match "Granted via", response.body
--- a/test/controllers/api/forward_auth_controller_test.rb
+++ b/test/controllers/api/forward_auth_controller_test.rb
@@ -186,7 +186,7 @@ module Api
      # Under default-deny the user must be in at least one group to access the app.
      # This rewritten test verifies that when an app's headers_config disables the
      # groups header, no x-remote-groups is sent regardless of memberships.
-      app = grant_everyone_access Application.create!(
+      grant_everyone_access Application.create!(
        name: "Headers Hidden", slug: "headers-hidden", app_type: "forward_auth",
        domain_pattern: "hidden.example.com",
        active: true,
@@ -559,7 +559,7 @@ module Api
    end

    test "should track failed attempts and eventually rate limit" do
-      cache = Rails.application.config.forward_auth_cache
+      Rails.application.config.forward_auth_cache

      # Make 50 failed requests (no session = unauthorized)
      50.times do
--- a/test/integration/csp_test.rb
+++ b/test/integration/csp_test.rb
@@ -32,6 +32,42 @@ class CspTest < ActionDispatch::IntegrationTest
      "inline theme script must carry the matching CSP nonce")
  end

+  test "signin page adds the OAuth redirect_uri host to form-action without 500ing" do
+    # A user must exist, otherwise /signin redirects to signup before the CSP
+    # branch runs.
+    User.create!(email_address: "csp_oauth@example.com", password: "password123")
+
+    app = Application.create!(
+      name: "CSP OAuth App",
+      slug: "csp-oauth-app",
+      app_type: "oidc",
+      redirect_uris: ["https://app.example.com/callback"].to_json,
+      active: true,
+      require_pkce: false
+    )
+
+    # An unauthenticated authorize request stores the full /oauth/authorize URL
+    # in the session and redirects to signin (oidc_controller.rb:202).
+    get "/oauth/authorize", params: {
+      client_id: app.client_id,
+      redirect_uri: app.parsed_redirect_uris.first,
+      response_type: "code",
+      scope: "openid"
+    }
+    assert_redirected_to signin_path
+
+    # Following to signin must reach allow_oauth_redirect_in_csp without raising.
+    # Regression: csp.form_action is a destructive getter, so reading it twice
+    # returned nil and `nil << host` raised NoMethodError -> 500.
+    follow_redirect!
+    assert_response :success
+
+    form_action = directive(response.headers["Content-Security-Policy"], "form-action")
+    assert_includes form_action, "'self'", "form-action must keep its default 'self'"
+    assert_includes form_action, "https://app.example.com",
+      "form-action must include the OAuth client's redirect_uri host"
+  end
+
  private

  def directive(csp, name)
--- a/test/test_helpers/session_test_helper.rb
+++ b/test/test_helpers/session_test_helper.rb
@@ -17,7 +17,11 @@ module SessionTestHelper
  # written under the old "empty allowed_groups = public" rule keep working.
  # New tests should attach groups explicitly to model real access intent.
  def grant_everyone_access(app)
-    everyone = (groups(:everyone) rescue Group.find_by(auto_assign: true))
+    everyone = begin
+      groups(:everyone)
+    rescue
+      Group.find_by(auto_assign: true)
+    end
    app.allowed_groups << everyone unless app.allowed_groups.include?(everyone)
    app
  end
Author	SHA1	Message	Date
Dan Milne	782e197d91	Fix access check form: use GET so results render Some checks failed CI / scan_ruby (push) Has been cancelled Details CI / scan_js (push) Has been cancelled Details CI / scan_container (push) Has been cancelled Details CI / lint (push) Has been cancelled Details CI / test (push) Has been cancelled Details CI / system-test (push) Has been cancelled Details Build and publish image / build (push) Has been cancelled Details The access check form POSTed and re-rendered :new with a 200 HTML response, which Turbo rejects ("Form responses must redirect to another location"), so the result panel never appeared. Since the check is a read-only query, switch to a GET form and fold the lookup into the new action. Results are now bookmarkable via the URL. Bump version to 0.16.2. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-21 15:42:57 +10:00
Dan Milne	020759bfb3	Fix invalid require-trusted-types-for CSP directive require-trusted-types-for only accepts 'script'; emitting 'none' produced an invalid directive that browsers rejected. Omit the directive entirely to leave Trusted Types unenforced (needed for WebAuthn). Bump version to 0.16.1. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-21 15:39:35 +10:00
Dan Milne	85f50bfc96	Add GitHub Actions workflow to build and publish image to GHCR Builds the production Docker image and pushes it to ghcr.io/dkam/clinch on pushes to main (edge + sha tags) and on v* release tags (vX.Y.Z, vX.Y, latest). amd64 only, with GHA layer caching. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-21 14:02:29 +10:00
Dan Milne	b55139eb1c	Fix Sentry config to use Sentry.init API Some checks failed CI / scan_ruby (push) Has been cancelled Details CI / scan_js (push) Has been cancelled Details CI / scan_container (push) Has been cancelled Details CI / lint (push) Has been cancelled Details CI / test (push) Has been cancelled Details CI / system-test (push) Has been cancelled Details The Sentry setup used a config.sentry.* accessor that sentry-rails has never provided, so booting with SENTRY_DSN set raised NoMethodError during environment load (e.g. db:prepare). The code only ran once a DSN was configured, which is why it surfaced in production now. Rewrites config/initializers/sentry.rb to call Sentry.init, the actual sentry-ruby API, and removes the duplicate broken block from production.rb. Verified production boots with SENTRY_DSN set (Sentry.initialized? == true) and that the no-DSN path still early-returns. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-21 13:57:26 +10:00
Dan Milne	8f578ed3f4	Upgrade Ruby to 4.0.5 Some checks failed CI / scan_ruby (push) Has been cancelled Details CI / scan_js (push) Has been cancelled Details CI / scan_container (push) Has been cancelled Details CI / lint (push) Has been cancelled Details CI / test (push) Has been cancelled Details CI / system-test (push) Has been cancelled Details Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-21 13:51:28 +10:00
Dan Milne	aa5736ddab	Update gems and fix lint to clear CI failures Bumps dependencies (jwt 3.2.0, puma 8.0.2, net-imap 0.6.4.1 and others via bundle update) to resolve bundler-audit advisories, and applies standardrb autofixes so the lint job passes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-06-21 13:51:23 +10:00
Dan Milne	49068aa344	Add tests Some checks failed CI / scan_ruby (push) Has been cancelled Details CI / scan_js (push) Has been cancelled Details CI / scan_container (push) Has been cancelled Details CI / lint (push) Has been cancelled Details CI / test (push) Has been cancelled Details CI / system-test (push) Has been cancelled Details	2026-06-15 08:22:23 +10:00
Dan Milne	07ea031b61	Remove hardcoded internal IP from production hosts allowlist Some checks failed CI / scan_ruby (push) Has been cancelled Details CI / scan_js (push) Has been cancelled Details CI / scan_container (push) Has been cancelled Details CI / lint (push) Has been cancelled Details CI / test (push) Has been cancelled Details CI / system-test (push) Has been cancelled Details 192.168.2.246 was redundant with the 192.168.0.0/16 regex already in the CLINCH_ALLOW_INTERNAL_IPS block, and baked a specific lab IP into the repo. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>	2026-06-11 23:55:02 +10:00
@@ -1 +1 @@
 .0.3
 .0.5