Skip to content
Merged
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
16 changes: 16 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ on:
permissions:
contents: read
pull-requests: write
# required by actions/upload-code-coverage (GitHub code-coverage merge protection)
code-quality: write

jobs:
test:
Expand Down Expand Up @@ -38,6 +40,20 @@ jobs:
path: coverage/
retention-days: 14

# Feeds the "Restrict code coverage" ruleset: GitHub compares the
# uploaded Cobertura data against the configured thresholds before
# allowing a merge. Runs even when the test step failed so GitHub still
# sees the (sub-threshold) numbers; fork PRs can't upload (no write
# token), which the action handles by exiting with a notice.
- name: Upload coverage to GitHub code quality
if: always()
uses: actions/upload-code-coverage@v1
with:
file: coverage/cobertura-coverage.xml
language: TypeScript
label: code-coverage/vitest
fail-on-error: false

- name: Comment coverage table on PR
if: always() && github.event_name == 'pull_request'
uses: actions/github-script@v7
Expand Down
86 changes: 86 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
# Contributing to Discussion Kit

Thanks for your interest in contributing! Bug reports, feature ideas, and pull requests are all welcome. This document explains how to get set up and what a pull request needs to be accepted.

## Getting started

```sh
git clone https://github.com/NotReeceHarris/discussion-kit.git
cd discussion-kit
npm install
npm run dev
```

The app is a fully client-side SvelteKit SPA over the GitHub Discussions GraphQL API — there is no backend or database. See the [README](README.md) for how everything fits together and how to point your dev instance at a repository with Discussions enabled.

### Useful commands

```sh
npm run dev # dev server
npm run check # type-check (svelte-check)
npm run test # unit tests (vitest)
npm run test:coverage # unit tests + coverage report
npm run build # static production build
npm run preview # preview the production build
```

## Branch naming

All new branches **must** follow this naming schema:

| Prefix | Use for |
| --- | --- |
| `feature/*` | New features and enhancements (e.g. `feature/profiles`) |
| `develop/*` | Ongoing development work such as refactors and optimisations (e.g. `develop/optimisation`) |
| `bug/*` | Bug fixes (e.g. `bug/profile-comment-leak`) |

Branches that don't match one of these prefixes will be asked to rename before review.

## Pull requests

1. **Fork** the repository and create your branch from `main`, named per the [branch naming schema](#branch-naming) above.
2. Make your changes, keeping the style of the surrounding code (tabs, Svelte 5 runes, Tailwind utility classes).
3. Add or update **unit tests** for any logic you touch (see below).
4. Make sure everything passes locally before opening the PR:
```sh
npm run check
npm run test:coverage
```
5. Open the pull request with a clear description of what it changes and why.

## ⚠️ 100% test coverage is required

**Pull requests will not be accepted unless test coverage is 100%.**

All logic modules (`src/lib/**/*.ts` and `oauth-proxy/worker.js`) must sit at **100% statement, branch, function, and line coverage**. The thresholds are enforced in [`vitest.config.ts`](vitest.config.ts), so `npm run test:coverage` **fails** if any metric drops below 100% — and CI runs it on every pull request ([`test.yml`](.github/workflows/test.yml)), so a PR that lowers coverage cannot go green.

In practice this means:

- New functions, branches, and error paths in `src/lib` need tests in [`tests/`](tests/) covering **every** branch — including failure cases (bad responses, missing data, signed-out users).
- If you change existing behaviour, update the affected tests rather than deleting them.
- `.svelte` components are exempt from the unit-coverage gate (they're verified via the production build), but the logic behind them should live in `src/lib` where it *is* covered.

Run the coverage report locally to see exactly which lines are uncovered:

```sh
npm run test:coverage # prints a summary; full HTML report in coverage/index.html
```

## Reporting bugs & suggesting features

Open an [issue](https://github.com/NotReeceHarris/discussion-kit/issues) with:

- What you did, what you expected, and what actually happened
- Browser and OS if it's a UI issue
- Screenshots or console/network output where relevant

## Code style

- **TypeScript** everywhere; keep types accurate rather than reaching for `any`.
- **Svelte 5 runes** (`$state`, `$derived`, `$effect`, `$props`) — runes mode is forced for the project.
- **Tailwind CSS v4** utilities with the `fd-` design tokens (`bg-fd-card`, `text-fd-muted-foreground`, …) so custom themes keep working.
- Comments explain *constraints and why*, not what the next line does.

## License

By contributing, you agree that your contributions will be licensed under the same [license](LICENSE) as the project.
53 changes: 53 additions & 0 deletions SECURITY.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
# Security Policy

## Supported versions

Discussion Kit is deployed as a static site straight from `main`; there are no maintained release branches. Only the latest code on `main` receives security fixes — forks should pull regularly to stay patched.

| Version | Supported |
| --- | --- |
| `main` (latest) | ✅ |
| Older commits / forks | ❌ |

## Reporting a vulnerability

**Please do not open a public issue for security vulnerabilities.**

Instead, use one of these private channels:

1. **GitHub private vulnerability reporting** (preferred): [Report a vulnerability](https://github.com/NotReeceHarris/discussion-kit/security/advisories/new) on this repository.
2. **Email**: [me@reeceharris.net](mailto:me@reeceharris.net) with a subject starting with `[SECURITY]`.

Include as much of the following as you can:

- A description of the vulnerability and its impact
- Steps to reproduce or a proof of concept
- The affected component (forum app, OAuth proxy worker, CI workflows)
- Any suggested fix

You should receive an acknowledgement within a few days. Please allow a reasonable window for a fix to land and deploy before any public disclosure.

## Security model

Understanding what is and isn't in scope helps triage reports:

- **No backend, no database.** The forum is a static SPA talking directly to the GitHub GraphQL API. There is no server-side session, and GitHub enforces all permissions (posting, editing, deleting, moderation) server-side — the UI merely mirrors them.
- **Tokens stay in the browser.** GitHub tokens (PAT or OAuth) are stored in `localStorage` only and sent solely to `api.github.com`. They are never transmitted to any server operated by this project.
- **OAuth proxy** ([`oauth-proxy/worker.js`](oauth-proxy/worker.js)): a minimal Cloudflare Worker that exchanges an OAuth `code` for an access token. The client secret lives only in the Worker; it validates request `Origin` against an allowlist, and the app validates an OAuth `state` nonce client-side. Reports about secret exposure, origin-check bypasses, or token leakage here are very much in scope.
- **Content rendering.** Post and comment bodies are rendered by GitHub (`bodyHTML` / the Markdown API), which sanitises HTML. Bypasses that achieve script execution (XSS) through forum-rendered content are in scope.

### In scope

- XSS or HTML injection via discussions, comments, profiles, or config-driven content
- Leaking or exfiltrating a signed-in user's token
- OAuth flow weaknesses (state bypass, code interception, proxy origin bypass)
- Privilege issues purely client-side (e.g. the UI granting actions GitHub would reject is *cosmetic*, but anything that results in an unauthorised API action succeeding is in scope)
- Vulnerabilities in the CI/CD workflows that could compromise the deployed site

### Out of scope

- Vulnerabilities in GitHub itself (report to [GitHub's bug bounty](https://bounty.github.com/))
- Issues requiring a compromised browser, device, or GitHub account
- Rate limiting / denial of service against the GitHub API
- Social engineering of forum users or maintainers
- Reports from automated scanners without a demonstrated impact
2 changes: 1 addition & 1 deletion src/lib/cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import { forumConfig } from './config';

/** Bump when the shape of cached GraphQL data changes */
const VERSION = 1;
const VERSION = 3;
const BASE = 'dk:cache:';
const PREFIX = `${BASE}v${VERSION}:`;

Expand Down
5 changes: 2 additions & 3 deletions src/lib/components/CommentCard.svelte
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<script lang="ts">
import { resolve } from '$app/paths';
import { forumConfig } from '$lib/config';
import { addComment } from '$lib/github/api';
import { auth } from '$lib/github/auth.svelte';
Expand Down Expand Up @@ -62,9 +63,7 @@
loading="lazy"
/>
<a
href={author.url}
target="_blank"
rel="noreferrer"
href={resolve('/u/[login]', { login: author.login })}
class="text-sm font-medium hover:underline"
>
{author.login}
Expand Down
40 changes: 28 additions & 12 deletions src/lib/components/DiscussionRow.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -12,27 +12,38 @@
}: { discussion: DiscussionListItem; showCategory?: boolean } = $props();

const article = $derived(isArticle(discussion.body));
const href = $derived(resolve('/d/[number]', { number: String(discussion.number) }));
</script>

<a
href={resolve('/d/[number]', { number: String(discussion.number) })}
class="group flex gap-3 rounded-xl border border-fd-border bg-fd-card p-4 transition-colors hover:border-fd-ring/50 hover:bg-fd-accent/40"
<!-- The whole card is clickable via the title's stretched overlay link; the
author avatar/name are separate links (to the profile page) layered above
it, since anchors can't nest. -->
<div
class="group relative flex gap-3 rounded-xl border border-fd-border bg-fd-card p-4 transition-colors hover:border-fd-ring/50 hover:bg-fd-accent/40"
>
{#if discussion.author}
<img
src={discussion.author.avatarUrl}
alt={discussion.author.login}
class="mt-0.5 size-9 shrink-0 rounded-full border border-fd-border"
loading="lazy"
/>
<a
href={resolve('/u/[login]', { login: discussion.author.login })}
class="relative z-10 mt-0.5 shrink-0 self-start"
title="View @{discussion.author.login}'s profile"
>
<img
src={discussion.author.avatarUrl}
alt={discussion.author.login}
class="size-9 rounded-full border border-fd-border"
loading="lazy"
/>
</a>
{:else}
<div class="mt-0.5 size-9 shrink-0 rounded-full bg-fd-muted"></div>
{/if}

<div class="min-w-0 flex-1">
<div class="flex items-center gap-2">
<h3 class="truncate font-medium group-hover:text-fd-foreground">
{discussion.title}
<a {href} class="after:absolute after:inset-0">
{discussion.title}
</a>
</h3>
{#if article}
<span
Expand All @@ -50,7 +61,12 @@
<div class="mt-2 flex flex-wrap items-center gap-x-3 gap-y-1 text-xs text-fd-muted-foreground">
{#if discussion.author}
<span class="inline-flex items-center gap-1 font-medium text-fd-foreground/80">
{discussion.author.login}
<a
href={resolve('/u/[login]', { login: discussion.author.login })}
class="relative z-10 hover:underline"
>
{discussion.author.login}
</a>
<UserBadges login={discussion.author.login} />
</span>
{/if}
Expand All @@ -75,4 +91,4 @@
</span>
</div>
</div>
</a>
</div>
4 changes: 1 addition & 3 deletions src/lib/components/Header.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,7 @@
class="absolute right-0 z-50 mt-2 w-48 overflow-hidden rounded-xl border border-fd-border bg-fd-card py-1 shadow-xl"
>
<a
href={auth.viewer.url}
target="_blank"
rel="noreferrer"
href={resolve('/u/[login]', { login: auth.viewer.login })}
class="block px-3 py-2 text-sm hover:bg-fd-accent"
>
<span class="block font-medium">{auth.viewer.name ?? auth.viewer.login}</span>
Expand Down
Loading
Loading