Add documentation site and reader refinements #51
No reviewers
Labels
No labels
Compat/Breaking
Kind/Bug
Kind/Documentation
Kind/Enhancement
Kind/Feature
Kind/Security
Kind/Testing
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Reviewed
Confirmed
Reviewed
Duplicate
Reviewed
Invalid
Reviewed
Won't Fix
Status
Abandoned
Status
Blocked
Status
Need More Info
No milestone
No assignees
3 participants
Notifications
Due date
No due date set.
Reference
marvin8/yunjin!51
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/reader-tweaks-and-docs"
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?
Documentation site (#41)
mkdocs.yml+docs/), modelled on cang's: Home, Installation, Configuration (everyYUNJIN_*variable), CLI reference, Reader (index/aggregate/article pages, settings, ignore rules, keyboard shortcuts), and Container pages.deploy-docsjob publishes versioned docs on release:mike deploy --branch pagesfor the tag pluslatest, then uploads to git-pages atyunjin.marvin8.zone. First deploy creates thepagesbranch.Reader refinements
POST /hide-readtoggle; all query-parameter threading through links, forms and redirects is gone (#49).yunjin.marvin8.zone/<version>, and the version renders from package metadata instead of hardcoded text (#50).Closes #48
Closes #49
Closes #50
Closes #41
WuMing
Found 3 issue(s). See inline comments below.
@ -123,0 +127,4 @@needs: [gitleaks, checks]if: github.event_name == 'release' && !github.event.release.prereleasesteps:- uses: actions/checkout@v4security [LOW]
A08: The workflow uses actions/checkout with a mutable major-version tag instead of a pinned commit SHA. If the tag is moved or compromised, the action can run malicious code in the release pipeline. Pin to a full commit SHA and review updates.
@ -123,0 +137,4 @@- name: Verify tag is on mainrun: git merge-base --is-ancestor HEAD origin/main- uses: astral-sh/setup-uv@v5security [LOW]
A08: astral-sh/setup-uv is referenced by the mutable tag v5. Unpinned CI actions are an unverified pipeline step and can be replaced without review, leading to supply-chain compromise.
@ -123,0 +170,4 @@path: _site- name: Upload to grebedocuses: https://codeberg.org/git-pages/action@v2security [MEDIUM]
A08: The git-pages publishing action is loaded from a mutable v2 tag and receives a deployment token. A compromised or moved tag could exfiltrate the token or publish malicious pages. Pin the action to a full commit SHA and ensure the repository/action is trusted.
❓ Repo-convention call — A08 action pinning (duplicates of the #46 escalation)
.forgejo/workflows/ci.yml — actions/checkout@v4·.forgejo/workflows/ci.yml — astral-sh/setup-uv@v5·.forgejo/workflows/ci.yml — git-pages action@v2Duplicates of the findings escalated on #46 (comments 3534/3535) and left as a repo-wide decision.
actions/checkout@v4andastral-sh/setup-uv@v5pre-exist across this workflow and the sibling repos; the newdeploy-docsjob deliberately copies cang's proven release job, git-pages action included — the stated template for #41. Pinning yunjin's references alone would diverge from the fleet; pinning everywhere is a convention change beyond this PR's scope.Over to Marvin8: say the word and I'll pin all action references to commit SHAs in a follow-up PR (and propose the same for cang for parity).
Decision recorded: no action — repo-wide action pinning is overkill for a personal hobby project. The three A08 findings stand declined; no follow-up PR.
WuMing
Found 7 issue(s). See inline comments below.
@ -123,0 +164,4 @@uv run mike deploy --branch pages --push --update-aliases "$VERSION" latest- name: Checkout pages branchuses: actions/checkout@v4config [MEDIUM]
This second checkout leaves
persist-credentialsat its default (true), soactions/checkoutwrites the Forgejo auth header/token into_site/.git/config._siteis then handed to the publishing action (source: _site), so if that action uploads dotfiles/.git, the repository token is published to a public site. Fix by addingpersist-credentials: falseto this checkout step (the push already uses the extraheader configured in the earlier step), and/or delete_site/.gitbefore uploading.@ -123,0 +170,4 @@path: _site- name: Upload to grebedocuses: https://codeberg.org/git-pages/action@v2security [HIGH]
A08: The Codeberg git-pages action is pinned to mutable tag v2 and receives a privileged forge token. An attacker who moves that tag can execute arbitrary code and exfiltrate the token. Pin the action to a full commit SHA and use a least-privilege token.
@ -33,7 +36,8 @@ TDD, main-off-limits invariant) apply in full and are not repeated here.| `nox` | Runs `ty`, `ruff`, `complexipy`, `pytest` (the nox part of the CI checks job) | agent |docs [MEDIUM]
This row still describes the nox check set as only
ty,ruff,complexipy,pytest, but thechecksjob description updated above (lines 15–16) now also listspysentry,deptry,zaojun, and tests on 3.12 + 3.13 + 3.14. The row explicitly claims to describe "the nox part of the CI checks job", so the two descriptions of the same job disagree — update the row to match.@ -0,0 +5,4 @@## Requirements- Python **3.12+** (3.13 works too; supported range is `>=3.12,<3.14`)docs [HIGH]
The stated support range
>=3.12,<3.14contradicts the rest of this PR: AGENTS.md (line 39) now says Python>=3.12and that nox runs the tests on 3.12, 3.13 and 3.14, and README.md (line 39) says "Python 3.12+". If 3.14 is tested/supported, the upper bound here is wrong; if<3.14is still correct, the other documents are wrong. Reconcile the three so readers get one consistent requirement (and fix the odd "3.13 works too" phrasing).@ -0,0 +58,4 @@# Install with development dependenciesuv sync --all-groups# Run the full check suite (types, lint, complexity, tests)docs [LOW]
The inline comment for
uv run noxlists only "types, lint, complexity, tests", while README.md (line 86) and AGENTS.md (lines 15–16) describe the same suite as also including security (pysentry) and dependency (deptry) checks. Align the summary with those documents to avoid implying the suite is smaller than it is.@ -0,0 +49,4 @@- Reader: reader.md- Container: container.mdmarkdown_extensions:config [HIGH]
markdown_extensions:is indented two spaces, unlike its siblingsnav:(line 44) andextra:(line 82), so it is no longer a top-level key. YAML will try to read it as a continuation of thenavblock sequence and fail with a parser error (e.g. "expected , but found '?'"), somkdocs build/mike deploywill abort and the docs job will never publish. Dedentmarkdown_extensions:and its whole list (lines 52-80) to column 0 so it is a top-level key.@ -232,0 +265,4 @@response = redirect(next_path)if turn_on:response.set_cookie(security [LOW]
A05: The hide-read cookie is set with HttpOnly and SameSite but no Secure flag, so it may be transmitted over plaintext HTTP if the app is ever accessed without HTTPS. Add secure=True to ensure the cookie is only sent over HTTPS.
WuMing
Found 4 issue(s). See inline comments below.
@ -123,0 +150,4 @@run: |git config user.email "ci@forgejo"git config user.name "Forgejo Actions"git config http.https://forge.marvin8.zone/.extraheader "Authorization: token $FORGEJO_TOKEN"security [MEDIUM]
A02: The Forgejo token is persisted to the repository-local .git/config in plaintext. Later steps (including third-party actions) can read the file, and the token may be visible in process listings. Prefer a scoped per-command
git -c http.extraheader=...for the single push, or use a credential helper that does not persist the secret.@ -0,0 +85,4 @@### `ignore-tag` / `unignore-tag`Add or remove a tag from the ignore list. Ignored tags are excluded fromclustering.docs [MEDIUM]
"clustering" is stale terminology: the embedding/DBSCAN clustering pipeline has been removed (see AGENTS.md, "the legacy embedding + DBSCAN pipeline has been fully removed"), and docs/reader.md:77 says ignored tags are excluded "when the LLM groups articles". Reword to "excluded during LLM aggregation" so the CLI reference matches the rest of the docs.
@ -0,0 +38,4 @@- [Reader](reader.md) — the web interface, settings, and shortcuts- [Container](container.md) — running the published image## Licencedocs [LOW]
Inconsistent spelling of the same heading: this page uses British "Licence" while README.md line 90 uses "License". Pick one spelling (the docs otherwise favour British English, e.g. "colour", "sanitised") and apply it to both files.
@ -232,0 +269,4 @@_HIDE_READ_COOKIE,"1",max_age=_HIDE_READ_COOKIE_MAX_AGE,httponly=True,security [LOW]
A05: The
hide_readcookie is set withoutSecure. If the application is served over HTTPS, the preference cookie can still be transmitted over plaintext HTTP and observed or set by a network attacker. Addsecure=Trueand use the same flag when deleting the cookie.Review response — waves 344/345
✅ Fixed in
a5538ad.forgejo/workflows/ci.yml — pages checkout— the_sitecheckout now setspersist-credentials: false, so no token is written into the uploaded tree.docs/install.md — Python range— now "Python 3.12, 3.13, or 3.14", matching pyproject (>=3.12) and nox (3.12–3.14).AGENTS.md — nox rowanddocs/install.md — nox comment— both now list the full session set (ty,ruff,complexipy,pysentry,pyteston 3.12–3.14,deptry,zaojun).docs/cli.md — stale wording— "excluded during LLM aggregation".docs/index.md — spelling— unified on "License" (matches theLICENSEfile and README).🔴 Not actioned
mkdocs.yml — YAML claim— false positive:markdown_extensions:is a top-level key at column 0, aligned withnav:andextra:(the diff indentation WuMing saw comes from the diff gutter).mkdocs build --strictexits 0 — verified again on this push. The traceback behind the finding is plainyaml.safe_loadchoking on mkdocs'!!python/name:tags, which every mkdocs config with emoji extensions contains and which mkdocs itself resolves..forgejo/workflows/ci.yml — A08 pinning— duplicate of the declined pinning decision (recorded here): repo-wide action pinning is overkill for this hobby project.reader.py — Secure flag·reader.py — Secure flag (repeat)— declined:secure=Truewould silently break the preference on plain-HTTP self-hosted deployments (dev server, LAN, TLS-terminating reverse proxy), and the cookie carries only a hide-read view preference, not credentials..forgejo/workflows/ci.yml — extraheader— declined: the Configure-git step is cang's proven template verbatim; with thepersist-credentials: falsefix above, the uploaded_sitecheckout holds no credentials and the header only ever lives in the trusted job's own workspace. Same over-hardening bucket as the declined pinning wave — happy to revisit if the convention changes.WuMing
Found 1 issue(s). See inline comments below.
@ -123,0 +171,4 @@persist-credentials: false- name: Upload to grebedocuses: https://codeberg.org/git-pages/action@v2security [HIGH]
Third-party CI actions are referenced by mutable tags instead of immutable commit SHAs. This action is passed a Forgejo token (line 178), so a compromised or moved @v2 tag could execute arbitrary code in the release workflow and exfiltrate secrets. Pin actions/checkout@v4, astral-sh/setup-uv@v5, and this action to full commit SHAs. A08: Software and Data Integrity Failures.
.forgejo/workflows/ci.ymlline 173🔴 Duplicate — identical in substance to the finding answered here and here: repo-wide action pinning was considered and declined as overkill for this hobby project (decision). No further action.
@ -3,3 +3,3 @@**RSS Topic Aggregator and Feed Reader**> Cloud Brocade - Weaving together similar news topics from multiple RSS feeds into coherent, related aggregates.> Cloud Brocade — weaving similar articles from multiple RSS feeds intoWhy doesn't this mention the tools actual name "Yunjin" and its chinese characters. The english translation is nice to have and should stay.
README.mdline 5✅ Fixed in commit
0d45b2f— the tagline now reads: Yunjin (云锦, "Cloud Brocade") — weaving similar articles from multiple RSS feeds into coherent, related topic aggregates.WuMing
Found 3 issue(s). See inline comments below.
@ -123,0 +127,4 @@needs: [gitleaks, checks]if: github.event_name == 'release' && !github.event.release.prereleasesteps:- uses: actions/checkout@v4security [MEDIUM]
A08: Third-party CI action is referenced by the mutable tag v4 rather than a pinned full-length commit SHA. A compromised or republished tag can execute arbitrary code in the CI job. Pin actions to exact SHAs and enable tag/branch protection review.
@ -123,0 +137,4 @@- name: Verify tag is on mainrun: git merge-base --is-ancestor HEAD origin/main- uses: astral-sh/setup-uv@v5security [MEDIUM]
A08: Third-party CI action is referenced by the mutable tag v5 rather than a pinned full-length commit SHA. A compromised or republished tag can execute arbitrary code in the CI job. Pin actions to exact SHAs and enable tag/branch protection review.
@ -123,0 +150,4 @@run: |git config user.email "ci@forgejo"git config user.name "Forgejo Actions"git config http.https://forge.marvin8.zone/.extraheader "Authorization: token $FORGEJO_TOKEN"security [MEDIUM]
A02: The Forgejo token is expanded on the command line and persisted in git config as an extraheader. Shell expansion means the secret appears in the process argument list and can be logged if tracing is enabled. Use a credential helper or GIT_ASKPASS script that reads the token from the environment without placing it in argv or .git/config.
Duplicates of declined findings (wave 350)
.forgejo/workflows/ci.yml— checkout@v4 ·setup-uv@v5— A08 pinning, declined: repo-wide action pinning was considered and rejected as overkill for this hobby project (decision, answer, repeat)..forgejo/workflows/ci.yml— extraheader — A02, declined: cang's proven template verbatim; the uploaded_sitecheckout holds no credentials and the token only ever exists inside the trusted job's own workspace (answer).Identical to the findings answered above. No further action.