Release v0.6.0 #31
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "release/0.6.0"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Release v0.6.0 — Auth Expansion
Changes
Traditional Username/Password Authentication
Multi-Provider OIDC Support
Real-Time Status Updates (SSE)
Frontend Performance
Database Schema
Performance Demo & Speedtest
Security & Code Review Fixes
Checklist
go test ./...)References
Version: v0.6.0 — Auth Expansion (Gravity PM)
Code Review: Release v0.6.0 — Request Changes
Scope: 116 files, +16,398 / -914 lines, 23 commits
Overall architecture is solid — crypto fundamentals are correct (bcrypt cost 12,
crypto/randtokens, SHA-256 token hashing, HMAC-SHA256 sessions with constant-time comparison), cookie security is good (HttpOnly, SameSite=Lax), CSRF via Sec-Fetch-Site is a sound modern approach, and the integration test suite validates complete user journeys. However, there are several issues that need to be addressed before merge.❌ CRITICAL — Demo endpoints allow unauthenticated database writes and self-DoS
internal/handlers/demo.go+cmd/gashy/main.go:211-237/demo/api/*endpoints are registered without authentication andisPublicPathinmode.goexplicitly includes/demo. Any anonymous user can:CreateTestPage)CleanupTestPagetakes an arbitrary UUID with no ownership check (DELETE FROM pages WHERE id = $1)LoadTestspawns 50 goroutines × 50 iterations = 2,500 internal HTTP requests per call with no mutual exclusionFix: Gate write endpoints behind admin auth, restrict deletes to the showcase dashboard (
WHERE dashboard_id = $2), add a feature flag to disable the demo.❌ HIGH — XSS in SSE status HTML builder
cmd/gashy/main.go:641-667errMsgfrom status checks is interpolated into adata-status-tip='...'single-quoted attribute viafmt.Sprintf. TheescapeJSONStringhelper only escapes JSON chars, not HTML. A crafted error likefoo' onmouseover='alert(1)breaks out of the attribute.Same pattern in
components/partials/status.templ:187,226—templ.Raw()with color values and JSON error messages bypasses auto-escaping.Fix: Use
html.EscapeString()on the JSON before embedding in HTML attributes. Validate color values server-side (regex for hex/var patterns). Consider migratingbuildSSEStatusHTMLto a templ component.❌ HIGH — OIDC session data never persisted (provider logout broken)
internal/auth/oidc.go:227-237After
sessions.Create()sets the cookie onw,SetSessionData()reads the cookie fromr(the original request), which doesn't have it. The error is silently discarded (_ =). This meansprovider_idandid_tokenare never stored, so per-provider end-session logout will never work.Fix: Return session ID from
Createand pass it to aSetSessionDataByIDmethod, or add the new cookie to the request before callingSetSessionData.❌ HIGH — Magic login token consumed on GET
internal/handlers/reset.go:270-308MagicLoginis a GET handler that consumes the token and creates a session. Browser prefetchers, Slack/Discord link previews, and crawlers will consume the token before the user clicks.Fix: Two-step flow — GET renders a confirmation page with a POST form that actually consumes the token.
❌ HIGH — ProviderSetDefault is non-atomic
internal/handlers/admin_providers.go:358-371Two separate UPDATEs: clear all defaults, then set new default. If the second fails, the system has no default provider.
Fix: Wrap in a transaction or use
UPDATE ... SET is_default = (id = $1).❌ HIGH — Seed script references dropped columns
scripts/seed-test-dashboard.sqlAll 8
INSERT INTO pagesreferencetransition_in/transition_out, which no longer exist after migration 00030. Running this script will fail.⚠️ MEDIUM (12)
nextparameter —//evil.compassesnext[0] == '/'check (internal/auth/local.go:185-192). Useurl.Parseand verifyHostis empty.renderLoginError—fmt.Fprintfwith unescapedmessage(internal/auth/local.go:204). Usehtml.EscapeString().ForgotPasswordhas per-email throttle but magic links don't (internal/handlers/reset.go:217-267).resendTimesandattemptsgrow without cleanup goroutines (confirm.go:29,reset.go:43).internal/handlers/register.go:184-234).internal/handlers/admin_providers.go).internal/handlers/admin_providers.go:256-303).err.Error()returned to user (internal/handlers/setup.go:185,207).-- +goose Downin migrations 00027 and 00028 — prevents clean rollback.LoadSettingshits DB on every HTTP request with no caching (internal/middleware/mode.go:33).visibilitychange/beforeunloadcleanup (static/js/status-batch.js:131-174).static/js/page-prefetch.js).ℹ️ LOW (15)
No max password length (bcrypt truncates at 72 bytes), weak email validation (
@and.only), email existence leak on confirm resend, silent JSON unmarshal on invite metadata, impersonation middleware active in production, SSEEventschannel not closed on unsubscribe, no max SSE client count, no SSE reconnection backoff, stale polling IDs on DOM changes,animatingflag can get stuck, resize listener leak in page-tabs, Chart.js CDN without SRI, missing ARIA labels on tab controls, modals missingrole="dialog"and focus traps, redundant migration 00028.Test coverage gaps
All tests pass including race detector. Key gaps:
HandleLoginHTTP success path untested (session creation, remember-me,nextredirect),OIDCManager.ReloadFromDBat 0% coverage, admin endpoint authorization not negatively tested.Verdict
Requesting changes. The critical demo endpoint exposure and the HIGH-severity XSS/OIDC/magic-link issues need to be resolved before merge. The MEDIUM items are worth addressing in this release cycle but could be follow-up commits on the release branch.
PR #31 Review: Release v0.6.0 — Auth Expansion
41 commits | 116 files | +16.7k / -928 lines
Database Migrations: ✅ APPROVED
All 8 migrations (00023–00030) are clean:
Downsections (00027/00028 fixed in9f73b24)CASCADEfor tokens,SET NULLfor provider refs)Security Findings
CRITICAL (3)
1. CSRF bypass via Sec-Fetch-Site fallback —
internal/middleware/csrf.go:41-43When both
Sec-Fetch-SiteandOriginheaders are absent, the request is allowed unconditionally. Older browsers or crafted requests can bypass CSRF protection entirely on all state-changing endpoints. Consider requiring at least one security header to be present, or falling back to a session-bound check.2. Email enumeration via timing —
internal/auth/local.go:91-141Locked accounts return early (~85ms) vs non-existent accounts that hit bcrypt (~300-500ms). Attackers can differentiate valid from invalid emails by response time. Fix: always run bcrypt (even on a dummy hash) for non-existent accounts to normalize timing.
3. XSS via innerHTML in SSE/prefetch —
static/js/status-batch.js:63,73,141andstatic/js/page-prefetch.js:186,330,505Direct
innerHTMLassignment with server-provided HTML fragments. If backend data is ever tainted (e.g., stored XSS in status values), arbitrary script execution is possible. Consider usingtextContentfor safe content, or a sanitizer like DOMPurify for HTML fragments.WARNING (12)
auth/oidc.go:183handlers/reset.go:301-320config/config.go:65-68auth/local.go:52-66handlers/demo.gohandlers/sse.go:11-53auth/local.go:234-255//evil.com/pathpasses validationauth/local.go:186-190status.templ:194-196status-batch.js:194-200status.templ:372Code Quality: ✅ GOOD
%wwrappingcmd/gashy/main.go:93andhandlers/register.go:185,193Test Coverage: SOLID with GAPS
Well covered: Login flows, registration, email confirmation, password reset, magic links, rate limiting, token lifecycle, OIDC provider CRUD, CSRF validation.
Missing coverage:
Recommendation: Add
-raceflag to CI and add goroutine-based concurrency tests for auth code.Verdict
The PR is well-structured with good architecture, clean migrations, and solid test foundations. The 3 critical issues (CSRF fallback bypass, timing-based email enumeration, innerHTML XSS) should be addressed before merge. The warnings are worth tracking but are lower risk.
PR Review Followup
Responding to review posted on 2026-02-23T01:14:16Z (comment #712).
Triage Summary
Fixes Applied
The following findings have been resolved on this branch:
c2d7717dc7c7dbe7be6ac3e78df00a7b1a516c1cb11df70b7//evil.com— fixed indc7c7db41f09aee7be6ace7be6acAlready Addressed (Closed Without Changes)
Deferred to v0.7.0
The following low-severity items have been tracked for the next version:
Test Results
go test ./...— 0 failures)This followup was generated by the pr-followup skill.
PR Review — Updated Review (post-fix verification)
Verdict: Acceptable
Previous reviews posted on 2026-02-23T00:34:29Z and 2026-02-23T01:14:16Z. This review verifies the 11 applied fixes and performs a fresh deep analysis of the current branch state (50 commits, 117 files, ~16.8k insertions).
Fix Verification
//prefix10 of 11 fixes fully correct and complete.
Findings
Test Results
Human Review Items
3 items flagged for human review — see HUMAN REVIEW markers below.
Important Findings (13)
Security (5)
1. [IMPORTANT] OIDC nonce validation conditional bypass —
internal/auth/oidc.go:220The nonce check is skipped when the ID token lacks a nonce claim:
If an attacker replays a valid signed ID token that was issued without a nonce (e.g., from a different flow or a provider that doesn't include nonce by default), the check is bypassed entirely. The fix should require the nonce when one was requested:
2. [IMPORTANT] Stored XSS via admin CSS injection —
components/layouts/base.templ:73,76cssVarsStyle()andcustomCSSStyle()emit admin-provided CSS keys/values throughtempl.Raw()with no sanitization. A CSS variable value likered; }</style><script>alert(1)</script><style>:root{--x:breaks out of the<style>tag and injects arbitrary HTML/JS. This executes for ALL users viewing any page.HUMAN REVIEW: This requires admin access to exploit. In a single-admin homelab context, the admin already controls everything and this is low risk. In a multi-admin or multi-tenant deployment, this becomes a privilege escalation vector (one admin can inject persistent JS affecting all users). The decision to sanitize depends on the intended deployment model. If addressed: validate CSS var keys match
^--[a-zA-Z0-9_-]+$, escape</style>sequences in values, and consider a CSS sanitizer for custom CSS.3. [IMPORTANT] dummyHash should be a valid bcrypt hash —
internal/auth/local.go:30The current hash (
$2a$12$xxxx...) is 59 characters (valid is 60). Testing confirms Go's current bcrypt implementation still performs the full ~228ms computation, so timing protection works today. However, a future library update could short-circuit on malformed hashes and silently break the protection. Replace with a real pre-computed hash for robustness:4. [IMPORTANT] generateSessionID silently drops rand.Read error —
internal/auth/session.go:238_, _ = rand.Read(b)— if the system CSPRNG fails, the session ID defaults to all zeros. A broken entropy source is a critical security failure that should panic or return an error, not silently produce predictable session IDs.5. [IMPORTANT] HandleLogin success path and redirect validation untested —
internal/auth/local_test.goThe full
HandleLoginPOST flow (session creation,remember_meTTL, HTMXHX-Redirect, standard redirect) has no test. More critically, the?next=open redirect prevention logic (line 200) — which rejects//evil.com, absolute URLs, and non-/-prefixed paths — is security-critical with zero coverage. Add tests for: valid relative path, protocol-relative URL rejection, absolute URL rejection, emptynextdefault.Code Quality (5)
6. [IMPORTANT] Pervasive handler-level SQL (SoC violations)
Handlers across register.go, confirm.go, reset.go, admin_providers.go, setup.go, and dashboard.go execute raw SQL directly (
h.pool.QueryRow,h.pool.Exec). TheDashboardServicepattern is well-established but wasn't extended to user CRUD, OIDC provider management, invite/token operations, or status cache lookups. This makes handlers untestable without a database and mixes HTTP concerns with persistence.HUMAN REVIEW: This is an architectural pattern that affects many files. Refactoring to full service-layer extraction is substantial work. Consider whether to address now or track as a v0.7.0 improvement. The current code works correctly — this is about maintainability and testability.
7. [IMPORTANT] Multiple swallowed DB errors —
register.go:185,193,admin_providers.go:267-289Several
_ = h.pool.QueryRow(...)calls silently discard errors. Inregister.go, if the uniqueness check query fails,existsdefaults tofalse, allowing the INSERT to proceed (which will fail with a confusing cascade). Inadmin_providers.go, silent failures on safety-check queries (enabled count, user count, default count) can bypass deletion guards. At minimum, log errors and return 500.8. [IMPORTANT] Functions exceeding recommended length
main()IndexRegisterLoadTestScaleTestPageSaveSetupExtract sub-operations into named helpers.
main.goin particular would benefit fromregisterRoutes()andcreateHandlers()extraction.9. [IMPORTANT] Separate settings cache in mode middleware —
internal/middleware/mode.go:29-31EnforceModecreates a newDashboardServicewith its own settings cache, separate from the instance inmain.go. After a settings change, the middleware and handlers may see different cached values for up to 30 seconds. Pass the existingDashboardServiceinstance instead of creating a new one.10. [IMPORTANT] SaveSetup lacks transactional consistency —
internal/handlers/setup.go:132-208The OIDC setup path executes
INSERT INTO oidc_providersandUPDATE app_settingsas separate queries. If the provider insert succeeds but the settings update fails, the system is left in an inconsistent state (provider exists but setup isn't marked complete).Frontend (3)
11. [IMPORTANT] DOM XSS vector in status tooltip —
components/partials/status.templ:310,326Inline script uses
innerHTMLwithlabelthat includesdata.errfrom server error messages. Whiledata.erris truncated at 40 chars, if status check errors can be influenced by a malicious upstream service, this is a DOM XSS path. UsetextContentfor the label portion.12. [IMPORTANT] Resize listener leak —
static/js/page-tabs.js:16-18init()adds a window resize listener that's never removed. After repeated htmx page swaps, old listeners accumulate. Store the handler reference and remove it in Alpine'sdestroy()lifecycle hook.13. [IMPORTANT] Event listener accumulation —
static/js/confirm-modal.js:12The
confirm-actioncustom event listener added ininit()is never cleaned up. If the component is destroyed and re-created during navigation, old listeners accumulate.Minor Findings (14)
register.go:288-352baseDatahelper duplicated verbatim across 4 handlers (register, confirm, reset, setup). Extract to shared function.dashboard.go:69-76invite.go:68-77admin_providers.go:334"Discovery failed: %v"may leak internal hostnames/DNS errors. Use generic message.services/dashboard.go:62-114LoadDashboardandLoadDashboardPrevieware near-identical. Extract sharedloadDashboardByID.oidc.go:58-61NewOIDCProviderreturns(nil, nil)for missing config — ambiguous API. Return sentinel error.status-batch.js:92,167target.innerHTML = el.innerHTMLre-parses sanitized content. Use node cloning instead for defense-in-depth.demo-tests.js:330,505,761innerHTMLwithout sanitization (admin-only pages — lower risk but inconsistent with other files).demo-tests.js:1026-1029getItemIds()/getSectionIds()callr.json()without checkingr.ok.toast.js:14,19confirm.templ:42<label>element.demo/layout.templ:60-72role="dialog",aria-modal="true".section.templ:49,133demo/layout.templ:120ChartWrapconcatenatestitleinto JS dispatch — would break on titles containing single quotes.Nitpick Findings (6)
local.go:193_ = metadatawith TODO comment. Wire into logging or remove.invite.go:98_ = userexists only to suppress unused-variable error.demo.go:352-356sectionNamesarray duplicated in two methods. Extract to package constant.mode.go:103-113isPublicPathreimplementsstrings.HasPrefix. Use it directly for clarity.services/dashboard.go:374s2for settings is confusing. Rename receiver tosvc.demo-tests.js(whole file)Test Coverage Gaps (Notable)
Well covered: Login flows, registration, email confirmation, password reset, magic links, rate limiting, token lifecycle, OIDC provider CRUD, CSRF validation. Integration tests cover complete user journeys.
Key gaps:
HandleLoginPOST success path (session creation, remember-me, redirect)?next=open redirect prevention (security-critical, zero coverage)DashboardHandler.Indexauthenticated render pathDashboardHandlermethods (Search,Status,StatusBatch,PageContent,SectionItems)InvalidateTokensandListTokensByTypein AuthTokenServiceMigration Review
All 8 migrations (00023-00030) are structurally correct:
9f73b24)scripts/seed-test-dashboard.sql) is compatible with current schemaHUMAN REVIEW: Migration 00030 drops
transition_in/transition_outfrompageswithout migrating existing values todashboards.transition. Rollback restores columns with defaults only. This is acceptable for schema evolution but worth acknowledging that it's a one-way data migration.Summary
The 11 security fixes from previous reviews are verified correct (10 fully, 1 partial). The codebase has solid cryptographic fundamentals, clean integration tests, and a well-structured authentication system. The remaining important findings are primarily:
No blocking findings. The PR is release-worthy with the advisory items tracked for follow-up.
@ai wrote in #31 (comment):
I don't like leaving this vector open, we should work around this with at least sanitation.@ai wrote in #31 (comment):
Slate this for 0.6.1 work. I do want to make sure this is clean and maintainable and testable without mixing concerns. You can also slot in other low priority work packages from this review into this followup minor version. @ai wrote in #31 (comment):
We should create a 0.6.1 work package to track the migration fixes. Migrations should be single-purpose in nature when possible. shouldn't be mixing tables and we should make sure that all migrations are reversible. We are currently pre-1.0 so that means we have the opportunity now to fix this because there are no live deployments.
PR Review Followup
Responding to review posted on 2026-02-23T01:55:36Z (comment #720) and maintainer feedback in comment #722.
Triage Summary
Fixes Applied
The following findings have been resolved on this branch:
56293223c43343be42865e768638552b933291f059b7daf19fa9cf74b4ef568Deferred to v0.6.1 — PR Review Followup
Version created: v0.6.1 — 8 work packages
Architecture (per maintainer):
Testing:
3. OP#1706: test: add HandleLogin success path and redirect validation tests
Migrations (per maintainer):
4. OP#1707: refactor: make migrations single-purpose and fully reversible
Minor findings (grouped):
5. OP#1708: refactor: extract duplicated helpers across auth handlers
6. OP#1709: fix: improve error handling and API clarity in auth subsystem
7. OP#1710: fix: frontend code quality (innerHTML safety, ARIA, event handling)
Nitpick findings (grouped):
8. OP#1711: chore: clean up dead code and naming inconsistencies
Test Results
go test ./...— 0 failures)This followup was generated by the pr-followup skill.
PR Review — Third Pass (fresh context, post-fix verification)
Verdict: Acceptable
Updated review — previous reviews posted on 2026-02-23. This review was conducted from a fresh context against the current branch state (59 commits, 117 files, ~17K insertions). All tests pass (0 failures). Focuses on findings not previously identified in the prior 5 comments.
Findings
Test Results
go test ./...— 0 failures)Human Review Items
1 item flagged for human review — see marker below.
Important Findings (10)
Security (4)
1. [IMPORTANT] Token validate-then-consume TOCTOU race —
internal/services/auth_token.go:83-121ValidateToken(SELECT) andConsumeToken(UPDATE) are two separate, non-transactional operations. Between validation and consumption, a concurrent request can validate and consume the same token. This affects all token flows: email confirmation (confirm.go:79-100), password reset (reset.go:198-222), magic link login (reset.go:331-339), and invite registration (register.go:98-103). For magic links specifically, this means two browser tabs could both create valid sessions from a single token.Suggested fix: Replace the two-step pattern with an atomic consume:
2. [IMPORTANT] Migration 00023 Down fails when local users exist —
internal/database/migrations/00023_auth_local_support.sql:40The down migration runs
ALTER TABLE users ALTER COLUMN oidc_subject SET NOT NULL, but any local-auth users will haveNULLinoidc_subject, causing a NOT NULL constraint violation. Rollback becomes impossible once local users exist. Per maintainer feedback in comment #722, pre-1.0 is the time to fix migrations.Suggested fix: Before
SET NOT NULL, backfill a placeholder:Or document that rollback past this point requires manual intervention.
3. [IMPORTANT] Missing index on
users.oidc_provider_idFK —internal/database/migrations/00026_oidc_providers.sql:32The
oidc_provider_idcolumn referencesoidc_providers(id)withON DELETE SET NULL, but no index exists on the column. The cascade operation and any provider-based user queries will require sequential scans onusers.Suggested fix: Add to the migration:
4. [IMPORTANT] PreviewStart sets cookie without authentication —
internal/handlers/dashboard.go:281PreviewStarthandlesPOST /preview/startand sets apreview_ascookie for any request, authenticated or not. While the cookie is consumed safely (used as a group filter), unauthenticated users should not be able to set cookies that influence dashboard rendering. Themodevalue is also not validated against known groups.Suggested fix: Add an auth check at the top of
PreviewStart— require at minimum an authenticated user, ideally a dashboard owner or admin.Code Quality (4)
5. [IMPORTANT] Compression middleware does not implement
http.Flusher—internal/middleware/compress.go:96-123gzipResponseWriterdoes not implementhttp.Flusher. The SSE path is correctly bypassed, but any future streaming handler not under/events/will get a false type assertion onw.(http.Flusher). This is a latent bug.Suggested fix: Implement
Flush()ongzipResponseWriter:6. [IMPORTANT]
renderDashboardtakes 18 positional parameters —internal/handlers/render.go:48This function signature is unwieldy and error-prone. All parameters are positional with no compiler help if the order is wrong (e.g., swapping two
stringparams compiles but breaks rendering).HUMAN REVIEW: This is a refactoring candidate. Consider a
DashboardRenderParamsstruct to make calls self-documenting. Could be bundled with the v0.6.1 architecture improvements (OP#1705).7. [IMPORTANT] Demo write handlers lack transactions —
internal/handlers/demo.go:205-295, 299-434CreateTestPageinserts a page, multiple sections, and bulk items with status cache as separate queries on the raw pool.ScaleTestPagehas the same pattern. If any step fails mid-way, orphaned rows remain. These are admin-only demo endpoints, but partial data can confuse the speedtest UI.Suggested fix: Wrap the entire create sequence in a database transaction.
8. [IMPORTANT] No tests for 3 new components
internal/middleware/compress.gointernal/middleware/static.gointernal/sse/hub.goThe SSE hub is particularly important to test with
-racegiven its concurrent access patterns.Frontend (2)
9. [IMPORTANT]
fetch()POST calls lack CSRF protection —static/js/edit-mode.js:146,171,181,248,290All direct
fetch()POST/PUT/DELETE calls inedit-mode.js(section reorder, item reorder, move item, rename section) do not include CSRF tokens. These bypass htmx's CSRF handling. The server usesSec-Fetch-Sitevalidation which should protect against cross-origin form submissions, butSec-Fetch-Siteis not universally supported and can be absent (Firefox ESR, embedded WebViews). The CSRF middleware's deny-by-default fallback (fixed inc2d7717) protects when headers are absent, but this means thesefetch()calls will be blocked on browsers that don't sendSec-Fetch-Site.Suggested fix: Add
Sec-Fetch-Site: same-originheader to fetch calls (browsers that support it set it automatically, so this is a no-op on modern browsers — but explicit headers ensure compatibility). Or better: include a CSRF token from a meta tag.10. [IMPORTANT] SSE write errors silently ignored —
internal/handlers/sse.go:50fmt.Fprintf(w, "event: status-update\ndata: %s\n\n", data)ignores the returned error. If the client disconnects butctx.Done()hasn't fired yet, the handler spins writing into a broken pipe until context cancellation.Suggested fix:
Minor Findings (7)
demo.go:398ScaleTestPagedivides bylen(allSectionIDs)— panics with division by zero if page has no sections. Add guard clause.auth/local.go:99-102authLimiter(10 req/s burst 20) is generous for auth endpoints.confirm.go:139-142ResendConfirmationreveals email verification status — "already confirmed" vs generic "if registered, link sent" enables email enumeration. Return same generic message for all cases.migrations/00026_oidc_providers.sql:10client_secretstored as plaintextTEXT. Consider application-level encryption with an envelope key from environment.middleware/compress.go:116-121flushHeaders()+gz.Close()may produce a non-empty gzip body for handlers that write no bytes. Track bytes written and skip compression if zero.dashboard.go:69-76resolveUserGroups(user) []stringhelper.services/dashboard.go:62-114LoadDashboardandLoadDashboardPrevieware near-identical. Extract shared query logic.Nitpick Findings (3)
auth/local.go:192-194_ = metadatawith TODO comment. Remove or implement.demo.go:245-249, 352-356sectionNamesarray duplicated in two methods. Extract to package-level constant.migrations/00029_toolbar_config.sql:2DEFAULT NULLis redundant in PostgreSQL.Previous Fixes Verified
All 20 fixes from prior review cycles (comments #716 and #732) remain correctly applied in the current branch. Specific verifications:
c2d7717) ✅be42865) ✅5629322) ✅1df70b7) ✅3c43343) ✅b7daf19) ✅291f059) ✅b4ef568) ✅fa9cf74) ✅Summary
The critical and high-severity security issues from prior reviews are all correctly resolved. The remaining important findings are primarily:
No blocking findings. The PR is release-worthy with these items tracked for follow-up (some align with existing v0.6.1 work packages).
@ai wrote in #31 (comment):
Good call, this should be bundled into work along with the rest of the follow-up in 0.6.1 from the PR comment. Also there are pipeline errors:
PR Review Followup
Responding to review posted on 2026-02-23T02:47:32Z (comment #737) and maintainer feedback in comment #738.
Triage Summary
Fix Applied
291b5ed--timeoutto 5m in.forgejo/workflows/ci.yamlto resolve lint job failureDeferred to v0.6.1
Per maintainer direction in comment #738, all review findings are tracked for v0.6.1. Version: v0.6.1 — PR Review Followup — now 19 work packages total.
New work packages created (11):
Existing work packages updated (3):
renderDashboard18 positional params finding (per maintainer #738)DEFAULT NULLin migration 00029Human Review Items (Unchanged)
renderDashboard18 positional parameters — bundled into OP#1705 per maintainer decisionTest Results
go test ./...— 0 failures)291b5ed)This followup was generated by the pr-followup skill.