Skip to content
Merged

fixes #835

Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
name: ci
on:
push:
pull_request:
jobs:
verify:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: pnpm/action-setup@v4
- uses: actions/setup-node@v4
with:
node-version: 24
cache: pnpm
- run: pnpm install --frozen-lockfile
- run: pnpm test
- run: pnpm check
3 changes: 2 additions & 1 deletion app/islands/loginWidget.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { css } from 'hono/css'
import { KeyRound } from 'lucide'
import * as v from 'valibot'
import { LucideIcon } from '../components/lucideIcon'
import { resolveSafeNextPath } from '../lib/safeRedirect'
import { creationOptionSchema, requestOptionSchema } from '../lib/webauthn'

const verificationSchema = v.object({
Expand Down Expand Up @@ -114,7 +115,7 @@ export const LoginWidget = ({ nextPath }: { nextPath: string | undefined }) => {
const handleLogin = async () => {
try {
await handleRegistration()
window.location.replace(nextPath ? nextPath : '/dashboard')
window.location.replace(resolveSafeNextPath(nextPath))
} catch (e) {
console.error('Error during login process:', e)
}
Expand Down
15 changes: 15 additions & 0 deletions app/lib/frontmatter.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { describe, expect, it } from 'vitest'
import { parseYaml } from './frontmatter'

describe('parseYaml', () => {
it('preserves colons inside values', () => {
expect(parseYaml('title: Foo: Bar\ndescription: a: b')).toEqual({
title: 'Foo: Bar',
description: 'a: b',
})
})

it('drops empty dash-list items', () => {
expect(parseYaml('tags:\n- a\n-\n- b')).toEqual({ tags: ['a', 'b'] })
})
})
9 changes: 7 additions & 2 deletions app/lib/frontmatter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,14 @@ export const parseYaml = (yaml: string) => {
if (!currentKey || !arrayBuffer) {
return {}
}
arrayBuffer.push(stripQuotes(line.slice(1).trim()))
const item = stripQuotes(line.slice(1).trim())
if (item !== '') {
arrayBuffer.push(item)
}
} else {
const [key, rawValue] = line.split(':').map((s) => s.trim())
const colon = line.indexOf(':')
const key = (colon === -1 ? line : line.slice(0, colon)).trim()
const rawValue = (colon === -1 ? '' : line.slice(colon + 1)).trim()
if (!key) {
return {}
}
Expand Down
40 changes: 40 additions & 0 deletions app/lib/safeRedirect.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { describe, expect, it } from 'vitest'
import { isSafeNextPath, resolveSafeNextPath } from './safeRedirect'

describe('isSafeNextPath', () => {
it('rejects absolute external URLs', () => {
expect(isSafeNextPath('https://evil.example/phish')).toBe(false)
})

it('rejects protocol-relative URLs', () => {
expect(isSafeNextPath('//evil.example/phish')).toBe(false)
})

it('accepts same-origin paths', () => {
expect(isSafeNextPath('/dashboard')).toBe(true)
})

it('rejects missing values', () => {
expect(isSafeNextPath(undefined)).toBe(false)
expect(isSafeNextPath('')).toBe(false)
})

it('rejects backslash and whitespace tricks', () => {
expect(isSafeNextPath('/\\evil')).toBe(false)
expect(isSafeNextPath('/dash board')).toBe(false)
})

it('rejects scheme URLs', () => {
expect(isSafeNextPath('javascript:alert(1)')).toBe(false)
})

it('accepts nested same-origin paths with query', () => {
expect(isSafeNextPath('/posts/20250101?x=1')).toBe(true)
})
})

describe('resolveSafeNextPath', () => {
it('falls back to /dashboard for external URLs', () => {
expect(resolveSafeNextPath('https://evil.example/phish')).toBe('/dashboard')
})
})
31 changes: 31 additions & 0 deletions app/lib/safeRedirect.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
export const isSafeNextPath = (value: string | undefined): value is string => {
if (!value) {
return false
}
if (!value.startsWith('/')) {
return false
}
if (value.startsWith('//')) {
return false
}
if (value.includes('\\')) {
return false
}
for (const ch of value) {
const code = ch.charCodeAt(0)
if (code <= 0x1f || code === 0x7f) {
return false
}
}
if (/\s/.test(value)) {
return false
}
return true
}

export const resolveSafeNextPath = (
value: string | undefined,
fallback = '/dashboard',
): string => {
return isSafeNextPath(value) ? value : fallback
}
3 changes: 3 additions & 0 deletions app/routes/images/[file].ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { HTTPException } from 'hono/http-exception'
import { createRoute } from 'honox/factory'
import * as v from 'valibot'
import { requireAuth } from '../../middlewares/requireAuth'
import { purgeImageCache } from '../../services/cache'

type imageFileName = `${string}-${string}-${number}:${number}.webp`

Expand Down Expand Up @@ -45,6 +46,7 @@ export const PUT = createRoute(
contentType: 'image/webp',
},
})
await purgeImageCache()

return c.json(result)
},
Expand All @@ -66,6 +68,7 @@ export const DELETE = createRoute(
} catch {
throw new HTTPException(500, { message: 'Failed to delete image' })
}
await purgeImageCache()
return c.json({ success: true })
},
)
Expand Down
6 changes: 4 additions & 2 deletions app/routes/login.tsx
Original file line number Diff line number Diff line change
@@ -1,20 +1,22 @@
import { createRoute } from 'honox/factory'
import { Meta } from '../components/meta'
import { LoginWidget } from '../islands/loginWidget'
import { resolveSafeNextPath } from '../lib/safeRedirect'
import { requireAuth } from '../middlewares/requireAuth'

export default createRoute(requireAuth, (c) => {
const nextPath = c.req.query('nextPath')
const safeNextPath = resolveSafeNextPath(nextPath)
const sessionId = c.get('sessionId')
if (sessionId) {
return c.redirect(nextPath ? nextPath : '/dashboard')
return c.redirect(safeNextPath)
}

return c.render(
<>
<Meta title="Login" />
<div>
<LoginWidget nextPath={nextPath} />
<LoginWidget nextPath={safeNextPath} />
</div>
</>,
{ heading: 'Login', isDashboard: true },
Expand Down
2 changes: 2 additions & 0 deletions app/routes/new.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { createRoute } from 'honox/factory'
import * as v from 'valibot'
import { EditPage, head } from '../components/editPage'
import { requireAuth } from '../middlewares/requireAuth'
import { purgePostCache } from '../services/cache'
import { addPost } from '../services/db'
import { parseMarkdown } from '../services/markdown'

Expand Down Expand Up @@ -85,6 +86,7 @@ export const POST = createRoute(
}

await addPost(c.env.DB, post, tags)
await purgePostCache()

return c.redirect('/dashboard', 303)
},
Expand Down
2 changes: 2 additions & 0 deletions app/routes/posts/[id]/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { ToC } from '../../../islands/toc'
import { ToCMobile } from '../../../islands/tocMobile'
import { getDatetime } from '../../../lib/getDatetime'
import { requireAuth } from '../../../middlewares/requireAuth'
import { purgePostCache } from '../../../services/cache'
import { deletePost, getPostByID } from '../../../services/db'
import { markdownToHtml } from '../../../services/markdown'

Expand Down Expand Up @@ -103,6 +104,7 @@ export const DELETE = createRoute(
async (c) => {
const { id } = c.req.valid('param')
await deletePost(c.env.DB, id)
await purgePostCache()
return
},
)
54 changes: 54 additions & 0 deletions app/services/cache.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import { describe, expect, it } from 'vitest'
import {
IMAGE_CACHE_NAME,
POST_CACHE_NAME,
purgeImageCache,
purgePostCache,
} from './cache'

describe('purgePostCache', () => {
it('deletes the post-cache store', async () => {
const deleted: string[] = []
const fakeCaches = {
delete: async (name: string) => {
deleted.push(name)
return true
},
}

await purgePostCache(fakeCaches)

expect(deleted).toEqual([POST_CACHE_NAME])
})
})

describe('purgeImageCache', () => {
it('deletes the image-cache store', async () => {
const deleted: string[] = []
const fakeCaches = {
delete: async (name: string) => {
deleted.push(name)
return true
},
}

await purgeImageCache(fakeCaches)

expect(deleted).toEqual([IMAGE_CACHE_NAME])
})

it('resolves when the cache backend is unavailable', async () => {
const failingCaches = {
delete: async (_name: string) => {
throw new Error("Failed to execute 'delete' on 'CacheStorage'")
},
}

await expect(purgePostCache(failingCaches)).resolves.toBeUndefined()
await expect(purgeImageCache(failingCaches)).resolves.toBeUndefined()
})

it('resolves when there is no caches global', async () => {
await expect(purgePostCache(undefined)).resolves.toBeUndefined()
})
})
34 changes: 34 additions & 0 deletions app/services/cache.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
export const POST_CACHE_NAME = 'post-cache'
export const IMAGE_CACHE_NAME = 'image-cache'

type CacheDeleter = Pick<CacheStorage, 'delete'>

const resolveCaches = (cachesLike?: CacheDeleter): CacheDeleter | undefined => {
if (cachesLike) {
return cachesLike
}
return typeof caches !== 'undefined' ? caches : undefined
}

const purgeCache = async (
name: string,
cachesLike?: CacheDeleter,
): Promise<void> => {
try {
await resolveCaches(cachesLike)?.delete(name)
} catch (e) {
console.error(`Failed to purge ${name}:`, e)
}
}

export const purgePostCache = async (
cachesLike?: CacheDeleter,
): Promise<void> => {
await purgeCache(POST_CACHE_NAME, cachesLike)
}

export const purgeImageCache = async (
cachesLike?: CacheDeleter,
): Promise<void> => {
await purgeCache(IMAGE_CACHE_NAME, cachesLike)
}
46 changes: 46 additions & 0 deletions app/services/db.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import { describe, expect, it } from 'vitest'
import { getPostsWithTag, getTags } from './db'

const collectSql = () => {
const seen: string[] = []
const stmt = {
bind: (..._params: unknown[]) => ({
all: async () => ({ results: [] }),
run: async () => ({}),
first: async () => null,
raw: async () => [],
}),
}
const fakeDb = {
prepare: (sql: string) => {
seen.push(sql)
return stmt
},
batch: async () => [],
exec: async () => ({}),
} as unknown as D1Database
return { fakeDb, seen }
}

describe('getPostsWithTag', () => {
it('scopes tag listings to public posts', async () => {
const { fakeDb, seen } = collectSql()

await getPostsWithTag(fakeDb, 'tech')

const sql = seen.join('\n')
expect(sql).toMatch(/"posts"\."public" = \?/)
})
})

describe('getTags', () => {
it('counts only tags of public posts', async () => {
const { fakeDb, seen } = collectSql()

await getTags(fakeDb)

const sql = seen.join('\n')
expect(sql).toMatch(/inner join "posts"/)
expect(sql).toMatch(/"posts"\."public" = \?/)
})
})
4 changes: 3 additions & 1 deletion app/services/db.ts
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,8 @@ export const getTags = async (db_binding: D1Database) => {
count: count(),
})
.from(postsToTags)
.innerJoin(posts, eq(postsToTags.postId, posts.id))
.where(eq(posts.public, true))
.groupBy(postsToTags.tagName)
.orderBy(desc(count()))

Expand All @@ -218,7 +220,7 @@ export const getPostsWithTag = async (
description: posts.description,
})
.from(posts)
.where(inArray(posts.id, postIdQuery))
.where(and(eq(posts.public, true), inArray(posts.id, postIdQuery)))
.orderBy(desc(posts.id))

return results
Expand Down
4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@
"preview": "wrangler dev --env preview",
"deploy": "wrangler deploy --env production",
"deploy:preview": "wrangler versions upload --env preview --preview-alias preview",
"check": "biome ci ./app vite.config.ts",
"check": "biome ci ./app vite.config.ts vitest.config.ts",
"fmt": "biome format --write ./app vite.config.ts",
"test": "vitest run",
"typegen": "wrangler types --env-interface=CloudflareEnv",
"setLocalDB": "LOCAL_DB_PATH=$(find .wrangler/state/v3/d1/miniflare-D1DatabaseObject -type f -name '*.sqlite' -print -quit)",
"setPreviewDB": "DB_ID='fe62b702-8f6b-4b85-81c0-e315529e6ca7'",
Expand Down Expand Up @@ -57,6 +58,7 @@
"es-module-lexer": "^3.0.3",
"postcss": "^8.5.29",
"vite": "^8.3.3",
"vitest": "^5.0.3",
"wrangler": "^4.148.0"
},
"packageManager": "pnpm@10.14.0+sha512.ad27a79641b49c3e481a16a805baa71817a04bbe06a38d17e60e2eaee83f6a146c6a688125f5792e48dd5ba30e7da52a5cda4c3992b9ccf333f9ce223af84748"
Expand Down
Loading
Loading