3 Commits

Author SHA1 Message Date
Dan Milne
c85d25c4b9 Untrack SECURITY_REVIEW_TODO.md and gitignore it
Some checks are pending
CI / scan_ruby (push) Waiting to run
CI / scan_js (push) Waiting to run
CI / scan_container (push) Waiting to run
CI / lint (push) Waiting to run
CI / test (push) Waiting to run
CI / system-test (push) Waiting to run
Keep the findings tracker local-only; it should not be published.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-28 23:09:17 +10:00
Dan Milne
1b0d323572 Bump version to 0.16.3
Some checks failed
Build and publish image / prepare (push) Has been cancelled
CI / scan_ruby (push) Has been cancelled
CI / scan_js (push) Has been cancelled
CI / scan_container (push) Has been cancelled
CI / lint (push) Has been cancelled
CI / test (push) Has been cancelled
CI / system-test (push) Has been cancelled
Build and publish image / build (amd64, linux/amd64, ubuntu-latest) (push) Has been cancelled
Build and publish image / build (arm64, linux/arm64, ubuntu-24.04-arm) (push) Has been cancelled
Build and publish image / merge (push) Has been cancelled
Ships the access-check GET-form fix (782e197) as a published image.
v0.16.2 was bumped before the version-bump build workflow existed, so it
never built; this bump triggers the build via the registered push trigger.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-21 18:29:39 +10:00
Dan Milne
d1d626c540 Rework build workflow to trigger on version bump + manual dispatch
Some checks failed
CI / scan_ruby (push) Has been cancelled
CI / scan_js (push) Has been cancelled
CI / scan_container (push) Has been cancelled
CI / lint (push) Has been cancelled
CI / test (push) Has been cancelled
CI / system-test (push) Has been cancelled
Port the build pipeline from the splat sibling project. Instead of
triggering on git tags, the image now builds when
config/initializers/version.rb changes on main — a version bump IS the
release — plus a workflow_dispatch button for manual builds.

Reads Clinch::VERSION, tags the image :vX.Y.Z, and moves :latest only
for non-pre-release versions. Also builds multi-arch (amd64 + arm64) on
native runners and stitches a manifest, replacing the amd64-only build.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-21 18:08:04 +10:00
4 changed files with 109 additions and 139 deletions

View File

@@ -1,22 +1,63 @@
name: Build and publish image
# Publishes the multi-arch image (amd64 + arm64) to GitHub Packages
# (ghcr.io/dkam/clinch) whenever config/initializers/version.rb changes on
# main — a version bump IS the release. Each arch builds natively (no QEMU); a
# merge job stitches them into one manifest tagged :vX.Y.Z (+ :latest for
# non-pre-releases).
#
# To cut a release: edit Clinch::VERSION in config/initializers/version.rb,
# commit, push. For a dev build: set a pre-release version (e.g. "1.1.0-dev") —
# it publishes :v1.1.0-dev but does not move :latest. Or run this workflow
# manually from the Actions tab.
on:
push:
branches: [ main ]
tags: [ 'v*' ]
paths:
- config/initializers/version.rb
workflow_dispatch:
# 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' }}
env:
IMAGE: ghcr.io/${{ github.repository }}
jobs:
build:
# Read the SemVer constant; decide whether this release moves :latest.
prepare:
runs-on: ubuntu-latest
outputs:
version: ${{ steps.version.outputs.version }}
latest: ${{ steps.version.outputs.latest }}
steps:
- name: Checkout code
uses: actions/checkout@v5
- name: Read version from config/initializers/version.rb
id: version
run: |
V=$(ruby -e "require './config/initializers/version'; puts Clinch::VERSION")
echo "version=$V" >> "$GITHUB_OUTPUT"
# A pre-release (e.g. 1.1.0-dev) publishes its own tag but not :latest.
if [[ "$V" == *-* ]]; then latest=false; else latest=true; fi
echo "latest=$latest" >> "$GITHUB_OUTPUT"
echo "Building v$V (move :latest = $latest)"
build:
needs: prepare
runs-on: ${{ matrix.runner }}
strategy:
fail-fast: false
matrix:
include:
- platform: linux/amd64
arch: amd64
runner: ubuntu-latest
- platform: linux/arm64
arch: arm64
runner: ubuntu-24.04-arm
permissions:
contents: read
packages: write # Required to push to GHCR
packages: write
steps:
- name: Checkout code
uses: actions/checkout@v5
@@ -31,26 +72,62 @@ jobs:
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
- name: Build and push by digest
id: build
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
platforms: ${{ matrix.platform }}
cache-from: type=gha,scope=${{ matrix.arch }}
cache-to: type=gha,mode=max,scope=${{ matrix.arch }}
outputs: type=image,name=${{ env.IMAGE }},push-by-digest=true,name-canonical=true,push=true
- name: Export digest
run: |
mkdir -p /tmp/digests
digest="${{ steps.build.outputs.digest }}"
touch "/tmp/digests/${digest#sha256:}"
- name: Upload digest
uses: actions/upload-artifact@v4
with:
name: digests-${{ matrix.arch }}
path: /tmp/digests/*
if-no-files-found: error
retention-days: 1
merge:
needs: [prepare, build]
runs-on: ubuntu-latest
permissions:
contents: read
packages: write
steps:
- name: Download digests
uses: actions/download-artifact@v4
with:
path: /tmp/digests
pattern: digests-*
merge-multiple: true
- 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: Create and push the multi-arch manifest
working-directory: /tmp/digests
run: |
tags="-t ${{ env.IMAGE }}:v${{ needs.prepare.outputs.version }}"
if [ "${{ needs.prepare.outputs.latest }}" = "true" ]; then
tags="$tags -t ${{ env.IMAGE }}:latest"
fi
docker buildx imagetools create $tags $(printf '${{ env.IMAGE }}@sha256:%s ' *)
- name: Inspect result
run: docker buildx imagetools inspect ${{ env.IMAGE }}:latest

3
.gitignore vendored
View File

@@ -70,3 +70,6 @@ yarn-debug.log*
# Ignore bootsnap cache
/tmp/cache/bootsnap*
# Local-only: do not publish the security findings tracker
SECURITY_REVIEW_TODO.md

View File

@@ -1,110 +0,0 @@
# 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`.

View File

@@ -1,5 +1,5 @@
# frozen_string_literal: true
module Clinch
VERSION = "0.16.2"
VERSION = "0.16.3"
end