Skip to content

fix(sub): randomize fresh panel subscription paths - #6375

Open
rokokol wants to merge 2 commits into
MHSanaei:mainfrom
rokokol:fix/random-subscription-paths
Open

fix(sub): randomize fresh panel subscription paths#6375
rokokol wants to merge 2 commits into
MHSanaei:mainfrom
rokokol:fix/random-subscription-paths

Conversation

@rokokol

@rokokol rokokol commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Seed distinct cryptographically random 16-character paths for base64, JSON, and Clash subscriptions when a fresh panel database is created
  • Persist the generated paths so restarts keep every published subscription URL stable
  • Leave existing installations unchanged so an upgrade never invalidates URLs that have already been distributed

Why

3x-ui currently ships the well-known /sub/ and /json/ subscription paths, and the Settings page itself reports both as security warnings. Although each subscription still has its own secret subId, a fixed service path makes the public subscription endpoints immediately recognizable and leaves every new panel with a warning that requires manual remediation.

Changing these paths after clients have been provisioned changes every subscription URL. Operators then have to rotate and redistribute those URLs just to remove an insecure default they did not choose. Generating panel-specific paths at database creation makes fresh installations safe by default and avoids that later operational churn.

The Clash path is randomized as well so all public subscription formats follow the same default. This is limited to fresh databases deliberately: silently changing paths during an upgrade would cause the exact URL rotation this change is intended to prevent.

The randomized prefix is defense-in-depth, not a replacement for the per-subscription subId.

Testing

  • make test-go
  • make lint-go
  • go build ./...
  • Verified all three paths are distinct between panels and remain unchanged after reopening the same database

Seed distinct cryptographically random paths for base64, JSON, and Clash subscriptions when a panel database is first created. Persist them so restarts keep published URLs stable while upgrades preserve existing settings.

Generated-by: OpenCode:gpt-5.6-sol
@github-actions

Copy link
Copy Markdown
Contributor

Code review

No blocking issues0 🔴 / 3 🟡 / 0 🟣

The core change is sound. seedRandomSubscriptionPaths runs after initModels() and before initUser(), so isUsersEmpty is still true on the first boot and false on every later one (db.go#L2152-L2166); FirstOrCreate keeps an operator's existing row, and random.NumLower draws from crypto/rand over [0-9a-z], so the value always satisfies the /xxxx/ shape normalizeSettingPaths expects. I traced the upgrade paths that worried me — panel SQLite import, pg_restore, and x-ui migrate-db (MigrateData truncates the destination inside the copy transaction, migrate_data.go#L118-L130) — and none of them reaches InitDB with an empty users table, so published URLs on existing installs do not rotate. The new test genuinely fails without the fix: absent the seed, no subPath row exists and the First at line 24 returns ErrRecordNotFound.


🟡 docs/ still advertises the old defaults

docs/content/docs/en/config/subscription.mdx#L20-L22 documents subPath with default /sub/, and the example URL two blocks down is https://<sub-host>:<sub-port>/sub/<sub-id>. Fresh installs no longer produce either, so an operator wiring a reverse proxy from the docs proxies a path the panel never serves. 12 files under docs/content/docs/ mention /sub/, and docs-ci.yml only fires on docs/**, so nothing flags the drift.

🟡 setting -reset puts the predictable paths back and nothing regenerates them

ResetSettings deletes every settings row (setting.go#L309-L314), so the three keys fall back to the in-code /sub/, /json/, /clash/ (setting.go#L103-L114). The new seed only runs when users is empty (db.go#L2156-L2161), which a reset never makes true, so a reset silently undoes exactly what this PR is for. The operator is not left blind — the panel's own warnDefaultSubPath banner reappears (SettingsPage.tsx#L177-L182) — so whether "reset means factory defaults" is the right answer here is your call, not a defect.

🟡 New test drops the CloseDB guard its siblings use

loadPaths closes the handle only on the happy path (db_seed_test.go#L34-L38); a t.Fatalf at line 25 or 28 returns with the package-level db still holding the file under t.TempDir(). All three other tests in this file register t.Cleanup(func() { _ = CloseDB() }) (lines 64, 126, 209), and internal/sub/build_urls_test.go#L16-L19 spells out why: Windows refuses to remove a still-open sqlite file during TempDir cleanup. Only bites on an already-failing run, but it turns one clear failure into two confusing ones.


Coverage

Reviewed head: 0f8348e92d725483eb89192bd244385e111616d8 — 2 files, +70/-0.

  • seedRandomSubscriptionPaths + InitDB call site — ordering, isUsersEmpty gate, FirstOrCreate semantics with a raw Where: correct; imports (random, gorm) already present.
  • Upgrade / data-loss paths — panel DB import, pg_restore, MigrateData, hostBoundSettingKeys (which excludes these three keys): no path rotates an existing install's URLs.
  • Wire-format impactinternal/sub reads the three paths from settings and registers its routes from them; no share-link or Clash/JSON body change, so the three link implementations stay in sync.
  • Existing tests — grepped every *_test.go for /sub/, /json/, /clash/; the only DB-backed assertion is TestNormalizeSettingPaths_RepairsLegacyValues, which this PR correctly repairs.
  • Migration rule — no schema or model change, so no hand-written migration is owed.
  • CI on this head — read the check runs: go-test, race, golangci, codegen, frontend, govulncheck, fuzz-smoke, postgres-durable-first and all 7 build matrix jobs concluded success.
  • Unverifiedpostgres-durable-first runs only -run 'PostgresCommitFailure' and the two *_Postgres schema tests, so the new test has never executed against PostgreSQL; this run reads code and never executes PR code, so nothing here was verified by running it.

Keep subscription paths unpredictable after a factory reset, close the test database on failure, and update the builder, OpenAPI, and localized docs to describe panel-specific paths instead of obsolete fixed defaults.

Generated-by: OpenCode:gpt-5.6-sol
@rokokol

rokokol commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

Addressed all three review notes in 7c733b10, and checked the surrounding docs for the same stale assumptions:

  • Settings reset: ResetSettings now deletes the old settings and creates fresh random subPath, subJsonPath, and subClashPath rows in one transaction. A reset therefore no longer falls back to the predictable paths this PR removes. Added TestResetSettingsRegeneratesSubscriptionPaths to check the path shape, distinctness, and that unrelated settings still return to their factory defaults.
  • Test cleanup: the fresh-panel test now defers CloseDB, so a failed assertion still releases the SQLite handle before t.TempDir cleanup on Windows.
  • Documentation: replaced the fixed /sub/, /json/, and /clash/ defaults with configured path names and <sub-path> examples in the EN/RU/FA/ZH subscription docs; updated the OpenAPI source and regenerated both specs plus the English API reference; synchronized the localized API references; corrected the stale subscription port from 10882 to 2096; and changed the interactive builder defaults to explicit /your-*-path/ placeholders.

Validation after the changes:

  • make test-go
  • make lint-go
  • go build ./...
  • frontend: lint, format check, typecheck, 1211 tests
  • docs: OpenAPI/reference generation, lint, format check, typecheck, 105 tests
  • both OpenAPI copies are byte-identical

Generated-by: OpenCode:gpt-5.6-sol

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant