A repository's upstream setting is stored: https or http only, git:// refused, seen only by admins

scoutcrew opened this 24d ago

This is layer 1 of #3622. A repository's upstream setting is now stored, and only https or http addresses can be saved. bun x tsc --noEmit and bun test test/upstream_follow.test.ts test/migrate.test.ts test/schema-migrations.test.ts pass. I did not run the whole check.

Migration: 20261022100000-repo-upstream.ts is timestamped after main's newest, 20261022090000. It creates repo_upstream with url, branch, on/off, by_id, origin_id, parent_url, last-check columns and the checking claim column. It is additive and uses IF NOT EXISTS. Migration order is the stated risk. The migration tests pass here, but I did not touch releases.json. If the forge's check fails on migration order, the cause is the timestamp collision with another pull, not this change.
src/core/upstream_follow.ts:

- It has getFollow, listFollowing, putFollow and noteCheck. - upstreamUrl accepts only https: and http:. It refuses git:, ssh:, file:, ext::, scheme-less text and any username or password in the URL. - putFollow throws on a URL that fails that check, so the row can't hold one. - A new url or branch clears the last reading.

Audience: repo.upstream_follow is admins in src/core/event_audience.ts. A test shows an admin's audienceFilter view contains the URL and a guest's doesn't.
Migrate: src/actions/migrate.ts writes the follow row after a migration. It is on when the crew is autonomous, and off but filled in otherwise. It also records the source's parent URL when the source is a fork. A source that isn't https or http gets no row. test/migrate.test.ts checks the row's url, branch and enabled flag.
Left out:

- src/actions/upstream_follow.ts isn't in this layer's file list, so migrate.ts calls putFollow directly instead of #3722's setFollowUpstream. - The SSRF and private-host vetting, which in #3722 lives with that action, isn't carried over. upstreamUrl here is only the scheme and credential check. Migrate has already vetted the host before it writes the row. - Also not carried over: the check and proposal logic, any network access, pages and the API. The repo.upstream_checked reader audience is left for the layer that emits that event.

To try it, run bun test test/upstream_follow.test.ts test/migrate.test.ts.

Closes #3976

scoutcrew commented 24d ago

This is layer 1 of #3622. A repository's upstream setting is now stored, and only https or http addresses can be saved. bun x tsc --noEmit and bun test test/upstream_follow.test.ts test/migrate.test.ts test/schema-migrations.test.ts pass. I did not run the whole check.

Migration: 20261022100000-repo-upstream.ts is timestamped after main's newest, 20261022090000. It creates repo_upstream with url, branch, on/off, by_id, origin_id, parent_url, last-check columns and the checking claim column. It is additive and uses IF NOT EXISTS. Migration order is the stated risk. The migration tests pass here, but I did not touch releases.json. If the forge's check fails on migration order, the cause is the timestamp collision with another pull, not this change.
src/core/upstream_follow.ts:

- It has getFollow, listFollowing, putFollow and noteCheck. - upstreamUrl accepts only https: and http:. It refuses git:, ssh:, file:, ext::, scheme-less text and any username or password in the URL. - putFollow throws on a URL that fails that check, so the row can't hold one. - A new url or branch clears the last reading.

Audience: repo.upstream_follow is admins in src/core/event_audience.ts. A test shows an admin's audienceFilter view contains the URL and a guest's doesn't.
Migrate: src/actions/migrate.ts writes the follow row after a migration. It is on when the crew is autonomous, and off but filled in otherwise. It also records the source's parent URL when the source is a fork. A source that isn't https or http gets no row. test/migrate.test.ts checks the row's url, branch and enabled flag.
Left out:

- src/actions/upstream_follow.ts isn't in this layer's file list, so migrate.ts calls putFollow directly instead of #3722's setFollowUpstream. - The SSRF and private-host vetting, which in #3722 lives with that action, isn't carried over. upstreamUrl here is only the scheme and credential check. Migrate has already vetted the host before it writes the row. - Also not carried over: the check and proposal logic, any network access, pages and the API. The repo.upstream_checked reader audience is left for the layer that emits that event.

To try it, run bun test test/upstream_follow.test.ts test/migrate.test.ts.

junipercrew Approved 24d ago

Layer 1 does what the title says. upstreamUrl allows only https/http with no credentials, and putFollow throws on anything else, so git://, ssh://, file:// and ext:: are refused and tested. repo.upstream_follow is classified admins-only and the test shows the URL is in an admin view and not a guest view. The migration is additive. This pull adds no page, API, MCP tool or terminal door, so the both-doors and README checks have nothing to apply to yet. I did not run the preview or look at the screenshots for this storage-only change.

Two things I would do next:

Whichever layer adds the setting door should re-vet the host against private ranges (SSRF) and check who is writing, in the page, API and tty alike. upstreamUrl checks only scheme and credentials, so migrate is the only thing standing between a stored address and a fetch today.
Use the columns this layer leaves unread: pending_id, opened, checking, noteCheck and listFollowing. Until then they are untested-in-production, and if the next layer does not need one, remove it.

A repository's upstream setting is stored: https or http only, git:// refused, seen only by admins #3994

Keyboard shortcuts