fix: filter repos by project is_public on browse frontend (#58) #59

Merged

forgejo_admin merged 2 commits from 58-fix-repos-public-filter into main 2026-03-02 05:43:40 +00:00

Conversation 2 Commits 2 Files changed 2 +225 -3

forgejo_admin commented 2026-03-01 22:33:58 +00:00

Owner

Copy link

Summary

• Filter repos by parent project's is_public on landing page (/browse/) and /browse/repos
• Add _apply_repo_public_filter helper following existing _apply_project_public_filter pattern
• Also apply _apply_project_public_filter to projects query on /browse/repos (was unfiltered)
• Repos with no project (NULL project_id) remain visible to all visitors

Changes

• src/pal_e_docs/routes/frontend.py: Added _apply_repo_public_filter helper that uses outerjoin through Repo.project and filters by Project.is_public when user is None. Applied to landing page and /browse/repos repos queries. Applied existing _apply_project_public_filter to /browse/repos projects query.
• tests/test_repo_public_filter.py: 6 new tests verifying repos and projects are filtered for unauthenticated visitors and visible for authenticated users on both /browse/ and /browse/repos.

Test Plan

• All 149 existing tests pass
• 6 new tests verify repos filtered by project visibility
• Manual: visit /browse/ unauthenticated, confirm private project repos hidden
• Manual: visit /browse/repos unauthenticated, confirm private project repos and projects hidden

Review Checklist

• Only touches frontend.py and the new test file
• Follows existing filter helper pattern
• Handles NULL project_id gracefully (outerjoin)
• ruff format and ruff check pass

Related Notes

• Forgejo issue #58
• plan-2026-02-28-knowledge-system-consolidation Phase 4 (Privacy Audit)
• issue-pal-e-docs-repos-public-filter

## Summary - Filter repos by parent project's `is_public` on landing page (`/browse/`) and `/browse/repos` - Add `_apply_repo_public_filter` helper following existing `_apply_project_public_filter` pattern - Also apply `_apply_project_public_filter` to projects query on `/browse/repos` (was unfiltered) - Repos with no project (NULL `project_id`) remain visible to all visitors ## Changes - `src/pal_e_docs/routes/frontend.py`: Added `_apply_repo_public_filter` helper that uses `outerjoin` through `Repo.project` and filters by `Project.is_public` when user is None. Applied to landing page and `/browse/repos` repos queries. Applied existing `_apply_project_public_filter` to `/browse/repos` projects query. - `tests/test_repo_public_filter.py`: 6 new tests verifying repos and projects are filtered for unauthenticated visitors and visible for authenticated users on both `/browse/` and `/browse/repos`. ## Test Plan - [x] All 149 existing tests pass - [x] 6 new tests verify repos filtered by project visibility - [ ] Manual: visit `/browse/` unauthenticated, confirm private project repos hidden - [ ] Manual: visit `/browse/repos` unauthenticated, confirm private project repos and projects hidden ## Review Checklist - [x] Only touches `frontend.py` and the new test file - [x] Follows existing filter helper pattern - [x] Handles NULL `project_id` gracefully (outerjoin) - [x] `ruff format` and `ruff check` pass ## Related Notes - Forgejo issue #58 - `plan-2026-02-28-knowledge-system-consolidation` Phase 4 (Privacy Audit) - `issue-pal-e-docs-repos-public-filter`

forgejo_admin added 1 commit 2026-03-01 22:33:58 +00:00

fix: filter repos by project is_public on browse frontend (#58) ... e37e886b16

Repos belonging to private projects were visible to unauthenticated
visitors on /browse/ and /browse/repos. Add _apply_repo_public_filter
helper that outer-joins through Repo.project and filters by
Project.is_public when user is None. Also apply _apply_project_public_filter
to the projects query on /browse/repos.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

forgejo_admin commented 2026-03-02 05:07:38 +00:00

Author

Owner

Copy link

PR #59 Review

BLOCKERS

None.

NITS

1. Missing test for repos with NULL project_id: The _apply_repo_public_filter helper explicitly handles Repo.project_id.is_(None) via the outerjoin, but no test seeds a repo without a project to verify it remains visible to unauthenticated visitors. Consider adding a test that creates a repo with no project_slug and asserts it appears on both /browse/ and /browse/repos for unauthenticated users. The logic is correct, but a regression test would lock it in.

2. _create_user session management: The _create_user helper in test_repo_public_filter.py wraps the session in try/finally with db.close(), but does not call db.rollback() on failure before closing. If db.commit() raises, the session is closed without rollback. In practice this is harmless with the in-memory SQLite test setup, but for consistency with defensive patterns, a try/except with rollback would be cleaner.

SOP COMPLIANCE

• Branch named after issue (58-fix-repos-public-filter references issue #58)
• PR body follows template-pr-body (Summary, Changes, Test Plan, Review Checklist, Related Notes)
• Related Notes references plan slug (plan-2026-02-28-knowledge-system-consolidation)
• Related Notes references issue slug (issue-pal-e-docs-repos-public-filter)
• Tests exist (6 new tests covering both routes, auth and unauth)
• No secrets, .env files, or credentials committed
• No unnecessary file changes (only frontend.py + new test file)
• Commit messages are descriptive (conventional commit format)
• Scope matches issue acceptance criteria

Code Quality Notes

• The _apply_repo_public_filter helper correctly follows the existing _apply_project_public_filter pattern.
• outerjoin(Repo.project) is the right choice: repos with NULL project_id are preserved, and each repo has at most one project, so no duplicate rows.
• The outerjoin for filtering and subqueryload for eager loading coexist correctly (subqueryload issues a separate query).
• Both frontend routes that query db.query(Repo) are covered (landing and browse_repos).
• The previously-unfiltered db.query(Project) on /browse/repos is also fixed -- good catch, as the repos.html template groups repos by project.

VERDICT: APPROVED

## PR #59 Review ### BLOCKERS None. ### NITS 1. **Missing test for repos with NULL `project_id`**: The `_apply_repo_public_filter` helper explicitly handles `Repo.project_id.is_(None)` via the outerjoin, but no test seeds a repo without a project to verify it remains visible to unauthenticated visitors. Consider adding a test that creates a repo with no `project_slug` and asserts it appears on both `/browse/` and `/browse/repos` for unauthenticated users. The logic is correct, but a regression test would lock it in. 2. **`_create_user` session management**: The `_create_user` helper in `test_repo_public_filter.py` wraps the session in `try/finally` with `db.close()`, but does not call `db.rollback()` on failure before closing. If `db.commit()` raises, the session is closed without rollback. In practice this is harmless with the in-memory SQLite test setup, but for consistency with defensive patterns, a `try/except` with rollback would be cleaner. ### SOP COMPLIANCE - [x] Branch named after issue (`58-fix-repos-public-filter` references issue #58) - [x] PR body follows `template-pr-body` (Summary, Changes, Test Plan, Review Checklist, Related Notes) - [x] Related Notes references plan slug (`plan-2026-02-28-knowledge-system-consolidation`) - [x] Related Notes references issue slug (`issue-pal-e-docs-repos-public-filter`) - [x] Tests exist (6 new tests covering both routes, auth and unauth) - [x] No secrets, .env files, or credentials committed - [x] No unnecessary file changes (only `frontend.py` + new test file) - [x] Commit messages are descriptive (conventional commit format) - [x] Scope matches issue acceptance criteria ### Code Quality Notes - The `_apply_repo_public_filter` helper correctly follows the existing `_apply_project_public_filter` pattern. - `outerjoin(Repo.project)` is the right choice: repos with NULL `project_id` are preserved, and each repo has at most one project, so no duplicate rows. - The `outerjoin` for filtering and `subqueryload` for eager loading coexist correctly (subqueryload issues a separate query). - Both frontend routes that query `db.query(Repo)` are covered (`landing` and `browse_repos`). - The previously-unfiltered `db.query(Project)` on `/browse/repos` is also fixed -- good catch, as the `repos.html` template groups repos by project. ### VERDICT: APPROVED

forgejo_admin referenced this pull request from a commit 2026-03-02 05:27:51 +00:00

test: add coverage for repos with NULL project_id (#59)

forgejo_admin added 1 commit 2026-03-02 05:27:51 +00:00

test: add coverage for repos with NULL project_id (#59) ... 67421c4b2c

Repos with no project assignment (project_id IS NULL) must remain
visible to unauthenticated users.  The outerjoin filter handles this
correctly but had no test locking the behavior.  Adds two tests
covering both /browse/ and /browse/repos.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

forgejo_admin commented 2026-03-02 05:29:38 +00:00

Author

Owner

Copy link

PR #59 Review

BLOCKERS

None.

NITS

None.

Code Review Details

Correctness -- The _apply_repo_public_filter helper is well-implemented:

• Uses outerjoin(Repo.project) so repos with NULL project_id are preserved (not excluded by an inner join).
• Filter condition (Repo.project_id.is_(None)) | (Project.is_public.is_(True)) correctly shows orphan repos and public-project repos while hiding private-project repos.
• No duplicate row risk -- Repo.project is many-to-one (at most one project per repo), so the outerjoin cannot fan out.
• No conflict between outerjoin (filtering) and subqueryload(Repo.project) (eager loading) -- they operate independently.

Completeness -- All browse frontend repo queries are covered:

• /browse/ (landing) -- filtered.
• /browse/repos -- both repos and projects queries filtered.
• /browse/projects/{slug} does NOT list Repo objects (only the project's own repo_url), so no leakage path there.
• The REST API (routes/repos.py) remains unfiltered, but that is a known pre-existing condition (no auth on the API at all, tracked in issue #2) and outside this PR's scope.

Tests -- 8 tests covering all cases:

• Unauthenticated: private repos hidden on landing and repos page.
• Authenticated: all repos visible on landing and repos page.
• Projects query on /browse/repos: private projects hidden for unauthenticated, visible for authenticated.
• Orphan repos (NULL project_id): visible to unauthenticated on both landing and repos page.
• Test helpers (_create_user, _login, _seed_repos, _get_test_db) follow existing patterns from test_project_schema.py and test_auth.py.

Security -- No data leakage paths identified. No secrets committed. Dummy test credentials (secret123) match existing test patterns.

Code Quality -- Clean, minimal diff. Follows the existing _apply_*_public_filter naming convention. Docstring is accurate. Only 2 files changed (source + tests), no scope creep.

SOP COMPLIANCE

• Branch named after issue (58-fix-repos-public-filter -> issue #58)
• PR body follows template-pr-body (Summary, Changes, Test Plan, Review Checklist, Related Notes)
• Related Notes references plan slug (plan-2026-02-28-knowledge-system-consolidation)
• Tests exist (8 new tests covering all cases)
• No secrets, .env files, or credentials committed
• No unnecessary file changes (scope is tight)
• Commit messages are descriptive

VERDICT: APPROVED

## PR #59 Review ### BLOCKERS None. ### NITS None. ### Code Review Details **Correctness** -- The `_apply_repo_public_filter` helper is well-implemented: - Uses `outerjoin(Repo.project)` so repos with `NULL project_id` are preserved (not excluded by an inner join). - Filter condition `(Repo.project_id.is_(None)) | (Project.is_public.is_(True))` correctly shows orphan repos and public-project repos while hiding private-project repos. - No duplicate row risk -- `Repo.project` is many-to-one (at most one project per repo), so the outerjoin cannot fan out. - No conflict between `outerjoin` (filtering) and `subqueryload(Repo.project)` (eager loading) -- they operate independently. **Completeness** -- All browse frontend repo queries are covered: - `/browse/` (landing) -- filtered. - `/browse/repos` -- both repos and projects queries filtered. - `/browse/projects/{slug}` does NOT list `Repo` objects (only the project's own `repo_url`), so no leakage path there. - The REST API (`routes/repos.py`) remains unfiltered, but that is a known pre-existing condition (no auth on the API at all, tracked in issue #2) and outside this PR's scope. **Tests** -- 8 tests covering all cases: - Unauthenticated: private repos hidden on landing and repos page. - Authenticated: all repos visible on landing and repos page. - Projects query on `/browse/repos`: private projects hidden for unauthenticated, visible for authenticated. - Orphan repos (NULL `project_id`): visible to unauthenticated on both landing and repos page. - Test helpers (`_create_user`, `_login`, `_seed_repos`, `_get_test_db`) follow existing patterns from `test_project_schema.py` and `test_auth.py`. **Security** -- No data leakage paths identified. No secrets committed. Dummy test credentials (`secret123`) match existing test patterns. **Code Quality** -- Clean, minimal diff. Follows the existing `_apply_*_public_filter` naming convention. Docstring is accurate. Only 2 files changed (source + tests), no scope creep. ### SOP COMPLIANCE - [x] Branch named after issue (`58-fix-repos-public-filter` -> issue #58) - [x] PR body follows `template-pr-body` (Summary, Changes, Test Plan, Review Checklist, Related Notes) - [x] Related Notes references plan slug (`plan-2026-02-28-knowledge-system-consolidation`) - [x] Tests exist (8 new tests covering all cases) - [x] No secrets, .env files, or credentials committed - [x] No unnecessary file changes (scope is tight) - [x] Commit messages are descriptive ### VERDICT: APPROVED

forgejo_admin referenced this pull request from a commit 2026-03-02 05:37:36 +00:00

test: add coverage for repos with NULL project_id (#59)

forgejo_admin force-pushed 58-fix-repos-public-filter from 67421c4b2c to 1858c04f1f 2026-03-02 05:37:36 +00:00 Compare

forgejo_admin merged commit eb84cda06f into main 2026-03-02 05:43:40 +00:00

forgejo_admin deleted branch 58-fix-repos-public-filter 2026-03-02 05:43:40 +00:00

forgejo_admin referenced this pull request from a commit 2026-03-02 05:43:41 +00:00

fix: filter repos by project is_public on browse frontend (#58) (#59)

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

domain:backend

  

domain:devops

  

domain:frontend

  

status:approved

QA passed, awaiting merge approval

  

status:in-progress

Dev agent is actively working

  

status:needs-fix

QA found issues, back to dev

  

status:qa

PR submitted, awaiting QA review

  

type:bug

Bug fix

  

type:devops

Infrastructure/CI/config work

  

type:feature

New functionality

No labels

domain:backend

domain:devops

domain:frontend

status:approved

status:in-progress

status:needs-fix

status:qa

type:bug

type:devops

type:feature

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

forgejo_admin/pal-e-api!59

No description provided.