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
1 change: 1 addition & 0 deletions .claude/CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ assets/brand/ # Brand assets: SVGs, animated PNGs, icons (ICO/ICNS/PNG
- **Brand assets**: All brand files in `assets/brand/` (logo.svg, logotype.svg, icon.svg, brandkit.html + PNG/ICO/ICNS variants). Brand assets are **proprietary** (not MIT) — do not change license headers. Sidebar uses `<img>` tags for animated SVG logo + logotype (CSP-safe, blocks SVG script execution). Colour-blind mode passed via `?mode=cb-{colourBlindMode}` from settings store. Dark mode auto-detected via `@media(prefers-color-scheme)` in SVG CSS. Regenerate with `node scripts/generate-icons.mjs` (icons) and `node scripts/svg-to-apng.mjs` (animated PNGs). Copyright year: `2026` (dynamic via `scripts/update-copyright-year.sh`).
- **Third-party licence obligations when bundling**: The offline-installer build path (`release.yml` Step 8.5, `bundle_engines=true`) redistributes upstream engines and tools inside our signed installer. MIT/BSD/Unlicense/PSF components only need the upstream copyright + permission notice preserved alongside the binary. **LGPL components** (FFmpeg, MP4Box/GPAC) additionally need a written offer for source (component + exact version + upstream-source URL + fall-back contact, valid for three years) and the binary must remain user-substitutable — subprocess invocation already satisfies this. **GPL components** (get_iplayer, planned for M8) need the complete corresponding source shipped alongside the binary (or a three-year written offer); MeedyaDL's own MIT code is protected from GPL propagation by the "mere aggregation" exception because we subprocess-invoke rather than link. The harvesting + source-offer emission belongs in `release.yml` Step 8.5 itself so it can never be forgotten when a new component lands in `engines.toml` or `tool-versions.toml`. Full matrix + practical "written offer for source" template in `.claude/memory/project_third_party_licence_obligations.md`. Tracked in #802. Notes do **not** apply to the default tiny-installer path (`bundle_engines=false`) which only ships MeedyaDL's own code — engines copy themselves from PyPI / GitHub Releases onto the user's machine at first launch.
- **Licence compliance is enforced per-PR (#806)**: the `Licences` workflow (`.github/workflows/licences.yml`) runs on every PR to `main` and on `workflow_dispatch`. Three checks: (1) `npm run check:acknowledgements` confirms every direct dep in `Cargo.toml` / `package.json` is named in `ACKNOWLEDGEMENTS.md` (set-coverage drift); (2) `npm run check:upstream-licences` confirms each direct dep's actual upstream-declared licence string matches what `ACKNOWLEDGEMENTS.md` claims, catching upstream re-licensings between MeedyaDL releases (most commonly MIT → MIT/Apache-2.0 dual-licence flips, but also the rare permissive → copyleft change that would be a serious compliance risk); (3) `cargo-deny check licenses` enforces the permissive-only allowlist in `src-tauri/deny.toml` on the full transitive tree. The upstream-string check uses an SPDX-aware normaliser that treats `LGPL-3.0+` ≡ `LGPL-3.0-or-later` and `MIT/Apache-2.0` ≡ `MIT OR Apache-2.0` as equivalent (advisory, not blocking). The `Licences` workflow is in addition to (not a replacement for) the existing cargo-deny step inside `ci.yml::backend` — duplicated here so the compliance picture lives in one workflow a maintainer can read in isolation. Run locally via `npm run check:legal` (umbrella over both Node scripts).
- **PR security heuristics run per-PR**: the `PR Security Checks` workflow (`.github/workflows/pr-security.yml`) runs on every PR to `main` / `release-candidate` / `beta` / `alpha` and on `workflow_dispatch`. **All checks are non-blocking advisory** — the merge gate stays with `ci.yml` (which already hard-gates `cargo clippy -D warnings`, `cargo test`, `cargo-deny`, `tsc`, `eslint`, and CodeQL for JS/TS + Actions); this workflow adds the *heuristic* layer those gates don't cover and posts a **single upserted PR comment** (edited in place by hidden marker `<!-- meedyadl-pr-security -->`, not a new comment per push). Adapted from the WebMS-Intra `pr-security.yml` to MeedyaDL's Rust/TS/Tauri stack. Eight checks: (1) gitleaks CLI secrets scan (working tree + commits since base, `--redact`, findings surfaced in the comment, no SARIF upload so the permission surface stays `contents:read` + `pull-requests:write`); (2) Rust subprocess shell-interpolation grep (`Command::new("sh")` / `.arg("-c")` — enforces the "no `sh -c`" invariant); (3) `unsafe` Rust blocks/fns in non-test changed code; (4) dangerous frontend sinks (`eval` / `new Function` / `dangerouslySetInnerHTML` / `innerHTML=`); (5) hardcoded absolute paths (`/Users/`, `/home/<user>/`, `C:\`); (6) unpinned GitHub Actions (`uses: org/repo@<tag>` not a 40-hex SHA — handles both `- uses:` and bare `uses:` forms, exempts `./local` and `docker://`); (7) sensitive/proprietary path touches (`assets/brand/` is PROPRIETARY, `src-tauri/capabilities/`, `tauri.conf.json`, `.github/workflows/`, signing/entitlements); (8) cross-source consistency via `tools/audit-checks/`. Checks 2–7 scan only PR-changed files; check 8 validates whole-repo state. The two consistency scripts — `check_ipc_commands.py` (every `#[tauri::command]` is registered in `lib.rs` `generate_handler![]` AND every frontend `invoke('x')` targets a registered command — the runtime "command not found" class) and `check_codec_registry.py` (every meta-codec `resolves_to` target is a real codec section AND every audio `services.gamdl` flag is a kebab-case `SongCodec` variant) — are pure cross-source reference validators (the MeedyaDL analog of WebMS's `check_route_targets.py` / `check_sql_columns.py`), exit 0 by default and 1 under `--strict`, report ` • path:line — msg` bullets the workflow greps for, and **must stay zero-finding on a clean tree** (add a negative test when changing them). Run locally: `python3 tools/audit-checks/check_ipc_commands.py` / `check_codec_registry.py`. The matching **`.github/pull_request_template.md`** carries the manual security-review checklist (subprocess safety, IPC contract registration, keychain/redaction, CSP/capabilities, licensing, proprietary-asset headers) that a grep can't cover.
- **Git operations**: Do NOT auto-commit or auto-push. Only edit files — let the user control git operations.
- **Documentation maintenance**: When adding features, modifying settings, changing commands/services, or altering UI — update ALL affected markdown files (README.md, Project_Plan.md, CHANGELOG.md, CLAUDE.md, help/*.md). This includes version numbers, file counts, feature lists, project structure trees, and help topic cross-references. Project_Plan.md serves as both the plan and status tracker (PROJECT_STATUS.md was consolidated into it).
- **GitHub Issue tracking**: For every task (features, bug fixes, enhancements, security fixes): (1) Create a GitHub Issue if one doesn't exist (`gh issue create`); (2) Close with completion comment when done (`gh issue close --reason completed`); (3) Link parent/child issues in the body (e.g., "Depends on #107", "Part of #100"); (4) Add to "MeedyaDL Development" project (`gh project item-add 6 --owner MWBMPartners`); (5) Create follow-up issues for any future work identified during implementation.
Expand Down
1 change: 1 addition & 0 deletions .claude/memory/MEMORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@
- [v1.7 bumper bundle](project_v17_bumper_bundle.md) — 22-issue session on `feat/v1.7-bumper-bundle` (2026-05-17/18); 13 closed + 9 deferred-with-plan; 11 new unit tests; demonstrates the defer-with-plan vs close pattern
- [MeedyaSuite-core online-only](project_meedyasuite_core_online_only.md) — standing rule: when investigating MeedyaSuite-core integrations, always fetch from github.com/MWBMPartners/MeedyaSuite-core via `gh api`, never trust the local cargo checkout (pinned at Cargo.toml's branch ref, may lag main)
- [MeedyaSuite org migration (planned)](project_meedyasuite_org_migration.md) — pending consolidation of MWBMPartners/* repos under MeedyaSuite org; secrets inventory + transfer order pinned; tauri.conf.json already has multi-endpoint updater fallback (2026-05-22); GitHub does NOT support nested orgs (siblings only)
- [PR security heuristics + audit-checks](project_pr_security_checks.md) — `pr-security.yml` (8 non-blocking advisory checks: gitleaks, `sh -c`, unsafe, frontend sinks, hardcoded paths, unpinned actions, sensitive-path, consistency) + `tools/audit-checks/` (IPC contract + codec-registry, stdlib-only, zero-finding-on-clean discipline); adapted from WebMS-Intra; landed PR #905 (2026-06-03); note the unresolvable `setup-python` SHA still in `upstream-gamdl-watch.yml`
43 changes: 43 additions & 0 deletions .claude/memory/project_pr_security_checks.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
---
name: PR security heuristics + audit-checks (pr-security.yml, tools/audit-checks/)
description: How MeedyaDL's per-PR security heuristic gate and cross-source consistency scripts work, why they're shaped the way they are, and the gotchas hit standing them up — adapted from WebMS-Intra to the Rust/TS/Tauri stack
type: project
---
Landed 2026-06-03 on PR #905 (`ci: add PR security heuristics workflow + cross-source audit checks`), CI 12/12 green. Adapted from the WebMS-Intra `pr-security.yml` approach at the user's request ("add similar PR heuristics security checks to this project"). WebMS-Intra is PHP; MeedyaDL is Rust + TypeScript + Tauri, so this is an **adaptation, not a port** — the PHP-specific checks (PHP lint hard gate, mysqli SQL-injection, CSRF tokens, Psalm) were dropped and the heuristic layer re-aimed at MeedyaDL's own documented invariants.

## What exists

- **`.github/workflows/pr-security.yml`** — runs on PRs to `main` / `release-candidate` / `beta` / `alpha` + `workflow_dispatch`. **Every check is non-blocking (`continue-on-error`)** — the merge gate stays with `ci.yml` (clippy `-D warnings`, cargo test, cargo-deny, tsc, eslint, CodeQL). This adds only the heuristic layer those gates don't cover.
- **`tools/audit-checks/check_ipc_commands.py`** + **`check_codec_registry.py`** + **`README.md`** — zero-dependency Python cross-source validators, runnable locally.
- **`.github/pull_request_template.md`** — manual security-review checklist mapped to MeedyaDL invariants.
- **`.claude/CLAUDE.md`** — convention bullet under the "Licence compliance is enforced per-PR" neighbour.

## The 8 workflow checks

1. gitleaks CLI secrets scan (working tree + commits since base, `--redact`); findings surfaced in the PR comment, **no SARIF upload** (keeps perms at `contents:read` + `pull-requests:write`). 2. Rust subprocess shell-interpolation (`Command::new("sh")` / `.arg("-c")` — the "no `sh -c`" rule). 3. `unsafe` Rust in non-test changed code. 4. Dangerous frontend sinks (`eval` / `new Function` / `dangerouslySetInnerHTML` / `innerHTML=`). 5. Hardcoded absolute paths (`/Users/`, `/home/<user>/`, `C:\`). 6. Unpinned GitHub Actions (`uses: org/repo@<tag>` not a 40-hex SHA — handles both `- uses:` and bare `uses:` forms; exempts `./local` and `docker://`). 7. Sensitive/proprietary path touches (`assets/brand/` is PROPRIETARY, `src-tauri/capabilities/`, `tauri.conf.json`, `.github/workflows/`, signing/entitlements). 8. The two consistency scripts.

Checks 2–7 scan only PR-changed files; check 8 validates whole-repo state.

## The two consistency scripts (the WebMS checks-9-11 analog)

Both follow the WebMS pattern: validate that two sources which must agree have no compiler link between them ("code references something that doesn't exist in another source").

- **`check_ipc_commands.py`** — every `#[tauri::command]` under `src-tauri/src/` is registered in `lib.rs`'s `generate_handler![]`, AND every frontend `invoke('x')` literal targets a registered command. Catches the runtime "command not found" class (defined-but-unregistered compiles fine; a typo'd invoke target only fails when a user clicks the button).
- **`check_codec_registry.py`** — every `codecs.toml` meta-codec `resolves_to` target is a real concrete codec section, AND every audio `services.gamdl` flag is a kebab-case `SongCodec` variant. TOML is parsed with **targeted regex, not `tomllib`** (so the script is Python-version-agnostic and venv-free).

**Conventions to preserve:**
- Findings print as ` • path:line — message` bullets; the workflow greps for the `•` bullet to decide whether to surface a section. Keep that prefix.
- **Zero findings on a clean tree is mandatory.** Both were verified clean on the current tree and negative-tested (inject a bad `invoke()`, a dangling `resolves_to`, a bogus `gamdl=` → all caught). Add a negative test when you change a check — a check that cries wolf on day one gets ignored.
- Default exit 0; `--strict` exits 1 on a high-severity finding (for local pre-push hooks).

The README lists good next candidates: `engines.toml` ↔ `EngineCommandBuilder` impls, `tool-versions.toml` ↔ installed tools, i18n `t('key')` ↔ locale JSON. (A Rust↔TS `AppSettings` drift check is tempting but high-false-positive because of serde renames — validate carefully before adding.)

## Design choices worth remembering

- **One upserted comment, not one per push.** The comment carries a hidden marker `<!-- meedyadl-pr-security -->`; the workflow finds an existing marked comment via `gh api …/issues/{n}/comments` and PATCHes it (`jq -Rs '{body: .}' | gh api --input -`), else POSTs a new one. WebMS posts a fresh comment every push; this avoids that spam.
- **Self-referential advisory is by design.** Check 7 flags any PR touching `.github/workflows/`, so the PR that *added* `pr-security.yml` got flagged by `pr-security.yml` — correct behaviour (reviewers should confirm workflow/brand changes are deliberate), advisory, no action.

## Gotchas hit standing it up (2026-06-03)

- **`actions/setup-python@e348410041c5b0ca4452c8e292ca3936bac9ba7f # v6` is NOT a resolvable SHA.** My first pr-security.yml copied this pin from `upstream-gamdl-watch.yml:60` (a repo grep "confirmed" it was already in use). The job died in 2 s at action-resolution: *"Unable to resolve action … unable to find version e348410…"*. Fix: the audit scripts are stdlib-only (`re`/`sys`/`pathlib`), so **`setup-python` was removed entirely** — ubuntu runners' preinstalled `python3` runs them. **`upstream-gamdl-watch.yml` still carries the same bad pin** and will fail at its Python-setup step whenever that cron fires — a latent bug worth a follow-up (flagged to the user 2026-06-03).
- **Detecting CI *success* from a remote session is awkward.** Webhooks deliver CI *failures* and comments but never success/new-push/merge-conflict transitions. Unauthenticated `api.github.com` polling hits the shared-runner-IP rate limit fast (60/hr), there was no `GH_TOKEN` in the shell, and `send_later` wasn't available. The working pattern: rely on the failure-webhook for interim breakage, and arm a `Monitor` single-shot timer (`sleep N && echo`) to wake the session and re-query check-runs via the **authenticated GitHub MCP** (`pull_request_read get_check_runs`) once. Re-arm if still running.
105 changes: 105 additions & 0 deletions .github/pull_request_template.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
<!--
MeedyaDL — Pull Request Template

The "PR Security Checks" workflow (.github/workflows/pr-security.yml) runs
automated heuristic + cross-source-consistency scans on every PR and posts a
single advisory comment. This checklist is your MANUAL review pass — the part
a grep can't do. Tick each row consciously; delete sections that genuinely
don't apply.
-->

## Summary

<!-- 1-3 sentences: what does this PR do and why. Link the issue it closes. -->

## Scope of changes

- [ ] Rust backend (`src-tauri/src/`)
- [ ] React / TypeScript frontend (`src/`)
- [ ] IPC surface (new/changed `#[tauri::command]`)
- [ ] Settings schema (`models/settings.rs` + TS types)
- [ ] Engine / codec / tool config (`engines.toml`, `codecs.toml`, `tool-versions.toml`, `tags.toml`)
- [ ] CI / workflows (`.github/`)
- [ ] Documentation (README, CHANGELOG, CLAUDE.md, `Project_Plan.md`, `help/*.md`)
- [ ] Other: _____

## Test plan

- [ ] `cargo test` (in `src-tauri/`) passes
- [ ] `npm run type-check` and `npm run test` pass
- [ ] `cargo clippy -- -D warnings` is clean
- [ ] Tested the golden path manually (`cargo tauri dev`)
- [ ] Tested at least one edge / failure case

## Security review

**Tick each row consciously.** These mirror MeedyaDL's documented invariants
(see `.claude/CLAUDE.md`). The automated workflow flags some of these
heuristically, but a clean bot comment is not a substitute for this pass.

### Input handling

- [ ] URLs are validated as `http(s)://` before reaching a subprocess (`gamdl_service.rs` guard), and download URLs are domain-allowlisted (Apple Music / Classical / iTunes)
- [ ] Any new filesystem path goes through `validate_path_safe()` (rejects `..` traversal); no path is built from unsanitised user input
- [ ] User strings written to GAMDL `config.ini` pass through `sanitize_ini_value()` (strips `\n` / `\r`)
- [ ] Imported settings/manifests are length-capped and control-char-stripped (`sanitize_imported_settings()`)

### Subprocess safety

- [ ] All subprocess calls use parameterised `Command::new().arg()` — **no** `sh -c` / `bash -c` / `format!()`-into-shell patterns
- [ ] No user input reaches `eval` / `new Function` / a shell

### Output & UI

- [ ] No `dangerouslySetInnerHTML` / `innerHTML =` of untrusted or remote-derived strings; Markdown is rendered through `rehype-sanitize`
- [ ] No raw secret/credential values are rendered into the DOM or logged to the activity log

### Secrets / credentials

- [ ] No API keys, developer tokens, `.p8` keys, passwords, or wrapper auth tokens are committed (embedded keys come from `option_env!` build secrets only)
- [ ] Sensitive values are stored in the OS keychain (`keyring`), not in `settings.json`
- [ ] Wrapper URLs are passed through `redact_url_query()` before any logging
- [ ] No new file was added under a server/secret-managed path without justification

### Filesystem

- [ ] No hardcoded absolute paths (`/Users/…`, `/home/<user>/…`, `C:\…`) — paths derive from `app_data_dir` / `std::env::temp_dir()`
- [ ] New on-disk writes that must survive a crash use the atomic temp-then-rename pattern

### IPC contract

- [ ] Every new `#[tauri::command]` is registered in `tauri::generate_handler![]` in `lib.rs` **and** has a frontend wrapper (`src/lib/tauri-commands.ts`)
- [ ] Rate-limited commands (downloads, update checks, cookie imports) keep their limiter
- [ ] `python3 tools/audit-checks/check_ipc_commands.py` is clean

### Settings / registry consistency

- [ ] If `AppSettings` changed: `settings_version` bumped + `migrate_settings()` updated, and the TypeScript type mirror updated
- [ ] If `codecs.toml` changed: `python3 tools/audit-checks/check_codec_registry.py` is clean

### Dependencies / licensing

- [ ] New dependencies are permissively licensed (cargo-deny allowlist) and named in `ACKNOWLEDGEMENTS.md` (`npm run check:legal` passes)
- [ ] No GPL/copyleft code is *linked* into MeedyaDL's own MIT code (subprocess invocation is fine)

### CI / supply chain

- [ ] New GitHub Actions are pinned to an immutable 40-char commit SHA (not a `@vX` tag)
- [ ] No `[skip ci]` in commit messages (unless explicitly requested)
- [ ] `workflow_dispatch` inputs are consumed via `env:`, not interpolated directly into `run:` shell

### Proprietary assets

- [ ] `assets/brand/` files (if touched) keep their **proprietary** license headers — these are NOT MIT

## Documentation

- [ ] Updated all affected docs (README, CHANGELOG, CLAUDE.md, `Project_Plan.md`, `help/*.md`) — feature lists, settings, commands, file counts, structure trees

## Related issues

<!-- Closes #N, refs #N. Per repo convention every change has a tracking issue. -->

---

<sub>Checklist enforced by repo convention, not by CI. The automated **PR Security Checks** workflow adds heuristic scans on top of this manual review; both are advisory — the merge gate is `ci.yml`.</sub>
Loading
Loading