Skip to content

feat: add naive client and panel integration - #5962

Draft
atem32 wants to merge 1 commit into
MHSanaei:mainfrom
atem32:feat/naive-client
Draft

feat: add naive client and panel integration#5962
atem32 wants to merge 1 commit into
MHSanaei:mainfrom
atem32:feat/naive-client

Conversation

@atem32

@atem32 atem32 commented Jul 15, 2026

Copy link
Copy Markdown

Summary

This PR adds Naive client integration to 3x-ui with panel-managed lifecycle, UI actions, and runtime wiring for outbound usage through local SOCKS5.

Why

This implements the functionality discussed in #5961 and provides actual code for review instead of an architecture-only proposal.

Use case enabled:

  • admin can manage Naive client from panel, use generated Naive config + local SOCKS5 bridge.

Refs #5961

Type of change

  • Bug fix
  • New feature
  • Refactoring (no behavior change)
  • Documentation
  • Tests only
  • Build / CI / tooling
  • Other

Areas affected

  • Backend:
    • Naive lifecycle management (install/uninstall/start/stop/restart/version path)
    • Runtime integration for Naive-backed outbound flow
  • Frontend:
    • Naive status/actions and version modal behavior
    • Naive outbound form adapter credential normalization (prevents double-encoding)
  • Localization:
    • Naive-related keys and labels
  • Validation:
    • service-level sync tests and live probe verification (generate_204 via Naive SOCKS5)

@github-actions github-actions Bot added enhancement New feature or request go Pull requests that update Go code javascript Pull requests that update javascript code dependencies Pull requests that update a dependency file labels Jul 15, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Summary: This PR adds a substantial new subsystem: a panel-managed Naive proxy client with lifecycle management, a local-SOCKS5 runtime bridge into Xray, panel UI, and 13-locale i18n, plus a credential double-encoding fix for outbound probes. The backend design is generally careful (transactional DB sync, tag/proxy-URL validation, graceful process shutdown) and the new Go tests follow repo conventions. However the PR currently ships an 84 MB compiled binary and an out-of-sync frontend lockfile that will break CI, plus violations of this repo's hard rules and some duplicated logic. These need fixing before merge.

Critical / High / Repository hygiene
Location: x-ui (repository root, added file)
Problem: The PR adds a native compiled binary literally named x-ui, 88,370,312 bytes (about 84 MB), tracked by git as a binary blob. Almost certainly the author's local build output.
Why it matters: An opaque, unreviewable ~84 MB executable from an external contributor is a supply-chain risk and permanently bloats the repo once merged (a later delete commit does not remove it from history). .gitignore only ignores an x-ui/ directory, not a file literally named x-ui.
Recommendation: Drop this file from the branch history entirely and add a .gitignore rule for the literal x-ui filename.

Critical / High / Build
Location: frontend/package.json; frontend/package-lock.json (unchanged); frontend/pnpm-lock.yaml; frontend/pnpm-workspace.yaml
Problem: package.json adds five new direct dependencies (codemirror commands/language/lint/state/view) but package-lock.json was not regenerated (verified: its root entry has none of these). Instead the PR adds an unrelated pnpm-lock.yaml (8547 lines) and pnpm-workspace.yaml. This project's frontend uses npm; ci.yml's frontend job runs npm ci against frontend/package-lock.json (pnpm is only used for the separate docs/ site).
Why it matters: npm ci fails outright when package.json/package-lock.json are out of sync, so the frontend CI job will fail as-is.
Recommendation: Remove the pnpm files and regenerate package-lock.json via npm install in frontend/.

High / High / Convention and formatting
Location: internal/naive/process.go:61; internal/web/service/outbound/probe_http.go:288, 292-293, 769-771; frontend/src/lib/xray/outbound-form-adapter.ts:678; internal/web/controller/naive.go:27
Problem: Several new // line comments were added (process.go:61, probe_http.go:288 and the doc comment at 770-771, outbound-form-adapter.ts:678), violating the repo's explicit no-comments rule. Separately, naive.go:27 is indented with spaces instead of a tab, and probe_http.go has two spots with double blank lines (292-293, 769).
Why it matters: CLAUDE.md states the no-comments rule is non-negotiable and cannot be linter-enforced. The whitespace issues are real gofmt/gofumpt violations that make lint (part of make verify) will flag.
Recommendation: Remove the comments (rename identifiers if needed) and run gofmt/make lint before pushing.

Medium / High / Maintainability
Location: internal/web/service/xray_naive.go:13-58; internal/web/service/outbound/probe_http.go:770-819
Problem: injectNaiveOutbounds (build a socks outbound at the local naive listener, splice it in place of the naive placeholder) is implemented twice, nearly verbatim, in two packages, each with its own DB query.
Why it matters: A future fix applied to only one copy lets the live Xray config and the probe/test config silently diverge.
Recommendation: Factor the shared logic into internal/naive, which both call sites already import.

Medium / Medium / Security (input validation)
Location: internal/web/controller/naive.go:103-115 (logs handler); internal/naive/manager.go:169-197 (ReadLogLines)
Problem: GET /panel/api/naive/logs/:tag/:rows passes the raw tag param straight into ReadLogLines, which concatenates it into a filename with no call to naive.ValidateTag, unlike every other place a tag reaches the filesystem or a spawned process (e.g. Start() in process.go).
Why it matters: The endpoint requires admin auth, bounding impact, but it's the one place an unvalidated path segment reaches os.Open via string concatenation. Whether Gin's routing lets a caller smuggle traversal sequences past a single :tag segment isn't confirmed here, but the existing validator should clearly be applied.
Recommendation: Call naive.ValidateTag(tag) at the top of the logs handler and reject invalid tags.

Medium / High / Documentation (hard rule)
Location: frontend/src/pages/api-docs/endpoints.ts; internal/web/controller/naive.go:27-28
Problem: naive.go registers 8 routes but only 6 are documented in endpoints.ts/openapi.json. POST /naive/stop-all and POST /naive/binary/delete (both called by NaiveStatusCard.tsx) are missing.
Why it matters: CLAUDE.md requires an endpoints.ts entry for every new route.
Recommendation: Add the two missing entries and rerun make gen.

Medium / Medium / Correctness
Location: frontend/src/lib/xray/outbound-form-adapter.ts:663-671 (normalizeCredentialForUrl, used by naiveToWire)
Problem: This helper repeatedly decodeURIComponent's the username/password up to 3 times until stable, to undo double-encoding on wire round-trips. It cannot tell that case apart from a password an admin freshly typed that happens to contain a valid percent-encoded-looking substring (e.g. literal %40 or %2F): such input is silently decoded and its meaning changed before re-encoding. Not covered by the new tests, which only use plain alphanumeric values.
Why it matters: A credential containing such a sequence gets silently mangled, causing confusing auth failures against the upstream server.
Recommendation: Track "already wire-encoded" vs "freshly typed" explicitly instead of guessing from content.

Low / High / i18n; Low / Medium / Reliability
Location: frontend/src/pages/index/NaiveStatusCard.tsx:141 and NaiveVersionModal.tsx:101; internal/naive/manager.go:129-139; internal/web/controller/naive.go:78-92,117-131
Problem: Both components hardcode title="Naive Client" instead of t(...), unlike XrayStatusCard.tsx (fully translated), even though a sectionTitle key already exists. Separately, StopAll/restartAll/stopAll loop sequentially over instances, and each Process.Stop() waits up to 5s for graceful shutdown, so bulk operations over many instances could block the request for a long time.
Recommendation: Use t(...) for both titles; consider stopping/restarting instances concurrently if many Naive outbounds are expected.

Positive observations: The new Go tests follow repo stdlib-testing conventions well (t.Helper, t.TempDir, t.Cleanup), and syncNaiveOutboundsTx correctly wraps the DB sync in a transaction with process start/stop deferred until after commit. Tag/proxy-URL validation is consistently applied where outbounds are created or started, aside from the logs endpoint above.

Verdict: Request changes. The committed binary and the broken npm lockfile are independently blocking (the latter fails CI outright); combined with the hard-rule violations and duplicated logic, this needs another pass before merge. @MHSanaei

This review was generated automatically. A maintainer may follow up.

@atem32
atem32 force-pushed the feat/naive-client branch from 57bcf67 to a1d60d5 Compare July 15, 2026 10:40
Add a full naive proxy sidecar lifecycle and panel integration:

- internal/naive/: process manager, config builder, downloader,
  log writer, input validators, xray config injector
- internal/web/controller/naive.go: REST endpoints for status,
  install, restart-all, stop-all, logs, binary delete
- internal/web/service/xray_naive.go: xray config injection
- frontend: NaiveStatusCard, NaiveVersionModal, naive outbound form,
  Zod schema, outbound-form-adapter round-trip, API docs entries

Compliance fixes:
- No // line comments (repo rule)
- gofmt clean
- All 8 naive routes documented in endpoints.ts
- NaiveOutbound added to migrationModels() for cross-DB parity
- Tag validation in logs handler (path traversal guard)
- Credential encoding uses direct encodeURIComponent (no double-encode)
- i18n keys replace hardcoded titles
- waitDone helper makes Stop() await process exit on all platforms
@atem32
atem32 force-pushed the feat/naive-client branch from a1d60d5 to f0e2744 Compare July 15, 2026 10:41
@atem32

atem32 commented Jul 15, 2026

Copy link
Copy Markdown
Author

All review findings have been addressed in the latest push.

P0 — CI blockers

  • Removed accidental x-ui binary (84 MB) from the repo; added x-ui filename entry to .gitignore
  • Replaced pnpm-lock.yaml / pnpm-workspace.yaml with package-lock.json so the npm-based CI lockfile matches

P1 — Repository rules

  • Removed all // line comments from Go and TS files touched by this PR
  • Fixed gofmt violations (space indent → tab in naive.go, double blank lines in probe_http.go)
  • Added missing endpoints.ts entries for POST /panel/api/naive/stop-all and POST /panel/api/naive/binary/delete; also registered the naive.go base path in api_docs_test.go so TestAPIRoutesDocumented covers all 8 naive routes

P2 — Security / correctness

  • Added ValidateTag call in the logs handler to prevent path traversal via the :tag param
  • Replaced the guessing normalizeCredentialForUrl loop with direct encodeURIComponent — no more double-encoding of @, /, %
  • Replaced hardcoded "Naive Client" titles with t('pages.xray.naive.sectionTitle') in NaiveStatusCard and NaiveVersionModal

P3 — Maintainability

  • Extracted duplicated injectNaiveOutbounds logic into internal/naive/inject.go; both xray_naive.go and probe_http.go now delegate to naive.InjectNaiveOutbounds(cfg)
  • Added nil-DB guard in InjectNaiveOutbounds to avoid panic when called without an initialised database (caught by TestTestOutboundsPrevalidationAndOrdering)
  • Added NaiveOutbound to migrationModels() in migrate_data.go to satisfy TestMigrationModelsMatchPanelModels

Sequential stop/restart hardening (nice-to-have)

  • Extracted waitDone helper in process.go; Stop() now explicitly awaits process exit on both Windows (after Kill) and Linux (after SIGTERM, with Kill fallback) — eliminates the race window between stop and restart
  • Added TestWaitDoneClosed / TestWaitDoneTimeout unit tests for the new helper

Tests added

  • internal/naive/inject_test.go — 4 table-driven tests for InjectNaiveOutbounds
  • internal/naive/process_test.go — 2 tests for waitDone
  • frontend/src/test/outbound-form-adapter.naive.test.ts — 2 tests for special-character credential encoding

All of go test ./internal/naive/..., ./internal/database/..., ./internal/web/controller/..., ./internal/web/service/outbound/... pass. Frontend: typecheck, lint, test (naive suite), build, build-storybook all green.

@PUDGE133 PUDGE133 mentioned this pull request Jul 25, 2026
4 tasks
@MHSanaei
MHSanaei marked this pull request as draft July 28, 2026 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file enhancement New feature or request go Pull requests that update Go code javascript Pull requests that update javascript code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant