Skip to content

Commit e44baad

Browse files
committed
chore: trim code comments and drop operator identifiers
Comments: cut the long "why it is this way" blocks down to one or two lines each, mostly in the User-Agent template code added for 1.4.7 but also in the older modules touched below. Rationale that belongs in a design doc no longer sits in the source, and references to what the code used to look like are gone — git already has that. Net -245 lines, no behaviour change. OPSEC: removed the maintainer's own LAN addresses from source comments and docstrings. They had accumulated in burn-in war stories ("observed on <ip> during <version>") across the xray lock timeout, the DNS-pin rationale in config_gen, the resolv.conf clobber note in network_apply, the naive sidecar memory limit, and the ifupdown detection docstring. Each keeps the technical reason, minus the address. The private-range examples in the knowledge base, UI placeholders and the config defaults are product content and stay. Backend 867 passing, frontend 49, tsc and vite build clean.
1 parent 24bd967 commit e44baad

19 files changed

Lines changed: 206 additions & 461 deletions

backend/alembic/versions/018_add_useragent_templates.py

Lines changed: 13 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,13 @@
1-
"""Add UserAgentTemplate table and seed it with the former hardcoded presets.
1+
"""Add UserAgentTemplate table, seeded with the built-in UA presets.
22
3-
Until v1.4.6 the subscription UA catalogue was two module-level dicts in
4-
`app/api/subscriptions.py` — `_UA_MAP` (9 UA strings) and `_HAPP_PROFILES`
5-
(the per-OS Happ device tuples). Adding a preset for a new panel, or
6-
attaching an extra header the panel gates on, meant a code change and a
7-
redeploy.
3+
The presets are keyed by the same slugs already stored in
4+
`subscription.ua` and carry the same User-Agent strings, so every
5+
existing subscription resolves to an identical request after the
6+
upgrade — the rows are simply editable now.
87
9-
This migration moves the catalogue into `useragenttemplate` and seeds it
10-
with **exactly** those nine presets, keyed by the same slugs already
11-
stored in `subscription.ua`. So nothing changes behaviourally on upgrade:
12-
every existing subscription resolves to the identical User-Agent it used
13-
yesterday. The difference is that the rows are now editable — the whole
14-
point of the change is that `happ`'s app version, or `chrome`'s Chrome
15-
build number, can be bumped from the UI when a panel starts rejecting a
16-
stale fingerprint.
17-
18-
`builtin=1` on the seeded rows is informational only (it drives a badge
19-
in the UI and a louder delete confirmation). Built-ins are fully
20-
editable AND deletable; nothing re-seeds them, because resurrecting a
21-
row the operator deliberately deleted would be worse than an empty
22-
dropdown. The runtime keeps a hardcoded fallback map for exactly that
23-
case (`core/ua_templates.BUILTIN_UA_MAP`), so a deleted or renamed
24-
template degrades to the old UA instead of breaking a refresh.
25-
26-
Not a foreign key: `subscription.ua` stays a plain string. A dangling
27-
key has a well-defined fallback, whereas an FK would either block the
28-
delete or cascade into wiping subscriptions.
8+
`subscription.ua` stays a plain string rather than a foreign key: a
9+
dangling key falls back to `core/ua_templates.BUILTIN_UA_MAP`, whereas an
10+
FK would either block the delete or cascade into wiping subscriptions.
2911
3012
Revision ID: 018
3113
Revises: 017
@@ -43,24 +25,11 @@
4325
depends_on: Union[str, Sequence[str], None] = None
4426

4527

46-
# The seed rows are INLINED here rather than imported from
47-
# `app.core.ua_templates`, for two reasons:
48-
#
49-
# 1. **A migration is a historical snapshot.** If someone later adds a
50-
# tenth preset to `DEFAULT_UA_TEMPLATES`, an import would make this
51-
# migration retroactively seed different data than it did for every
52-
# install that already ran it. Migrations must not move.
53-
#
54-
# 2. **Deploy safety.** `docker-compose.yml` bind-mounts `./backend/app`
55-
# and `./backend/alembic` as two separate volumes, and `entrypoint.sh`
56-
# runs `alembic upgrade head` with `MIGRATION_STRICT=1` before the app
57-
# starts. A hot-deploy that copies the new `alembic/` but not the new
58-
# `app/` would hit an ImportError here and put the container in a
59-
# crash loop. With no app import there is nothing to get out of sync.
60-
#
61-
# `tests/test_ua_templates.py::TestMigrationSeedData` asserts these stay
62-
# byte-identical to `DEFAULT_UA_TEMPLATES`, so drift is a failing test
63-
# rather than a silent difference between fresh and upgraded installs.
28+
# Inlined rather than imported from `app.core.ua_templates`: a migration
29+
# is a historical snapshot, and `app/` and `alembic/` are separate bind
30+
# mounts, so an import would crash-loop the container on a deploy that
31+
# updated one before the other. `TestMigrationSeedData` pins these against
32+
# the runtime copy so the two cannot drift.
6433
_HAPP_NOTE = (
6534
"X-Device-* / X-Hwid headers are added automatically to match this profile."
6635
)
@@ -104,11 +73,9 @@ def upgrade() -> None:
10473
templates = op.create_table(
10574
"useragenttemplate",
10675
sa.Column("id", sa.Integer(), primary_key=True),
107-
# Stable slug referenced by `subscription.ua`.
10876
sa.Column("key", sa.String(), nullable=False),
10977
sa.Column("name", sa.String(), nullable=False),
11078
sa.Column("user_agent", sa.String(), nullable=False, server_default=""),
111-
# JSON object of extra request headers merged over the base set.
11279
sa.Column("headers", sa.String(), nullable=False, server_default="{}"),
11380
sa.Column("description", sa.String(), nullable=True),
11481
sa.Column(

backend/app/api/subscriptions.py

Lines changed: 4 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,6 @@
1818

1919
router = APIRouter(prefix="/subscriptions", tags=["subscriptions"])
2020

21-
# The UA catalogue lived in this module until v1.4.7 as two hardcoded
22-
# dicts (`_UA_MAP` + `_HAPP_PROFILES`). It now lives in the
23-
# `useragenttemplate` table — CRUD in `api/user_agents.py`, resolution
24-
# and the remaining code-side pieces (the built-in fallback map, and
25-
# Happ's X-* bundle whose `X-Hwid` must be derived per request) in
26-
# `core/ua_templates.py`.
27-
28-
2921
# ── CRUD ──────────────────────────────────────────────────────────────────────
3022

3123
@router.get("", response_model=List[SubscriptionRead])
@@ -96,18 +88,10 @@ async def refresh_subscription(
9688
sub = await session.get(Subscription, sub_id)
9789
if not sub:
9890
raise HTTPException(404, "Subscription not found")
99-
# Per-subscription mutex — concurrent calls return 409 instead of
100-
# spawning duplicate fetch tasks. Without this, two clicks within
101-
# a few hundred ms (UI double-click, scheduler tick overlapping a
102-
# manual refresh, two browser tabs etc.) used to fire two
103-
# background `_fetch_subscription` runs against the same row.
104-
# Each one would `delete all old nodes → insert new`, so the
105-
# second one racing the first could observe a half-deleted state
106-
# and import a partial node set, or both could land near-
107-
# simultaneously and corrupt `active_node_id` via duplicate
108-
# delete-then-create. Observed in the wild on 192.168.1.4 —
109-
# logs show 4 refreshes within 60s with one returning 57 nodes
110-
# instead of the canonical 1256.
91+
# Per-subscription mutex. Two refreshes racing (double-click,
92+
# scheduler tick overlapping a manual refresh, two tabs) could each
93+
# observe a half-deleted node set and import a partial one, or
94+
# corrupt `active_node_id`.
11195
if _is_refresh_active(sub_id):
11296
raise HTTPException(
11397
status_code=409,
@@ -211,11 +195,6 @@ async def _fetch_subscription_unlocked(sub_id: int) -> None:
211195
if not sub:
212196
return
213197

214-
# Resolve the full request fingerprint from the UA template the
215-
# subscription points at: User-Agent, the base Accept-* set, the
216-
# dynamic Happ X-* bundle where applicable, and any extra headers
217-
# the template declares. Precedence and merge order are
218-
# documented on `build_subscription_headers`.
219198
headers = await build_subscription_headers(session, sub)
220199

221200
content: str = ""

backend/app/api/system.py

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1193,13 +1193,10 @@ async def _collect_naive_bypass_dsts(session: AsyncSession) -> List[str]:
11931193
Domain names are resolved via ``socket.getaddrinfo`` (A records only).
11941194
11951195
Resolution runs OFF the event loop thread (``asyncio.to_thread``)
1196-
and with a 2-second per-host budget. The earlier sync version
1197-
blocked the entire FastAPI event loop when /etc/resolv.conf was
1198-
misconfigured — observed in the wild on 192.168.1.4 during 1.3.3
1199-
burn-in: every API endpoint that touched ``_auto_reload_xray``
1200-
(routing-rule CRUD, ``/system/restart``) stalled forever because
1201-
the underlying ``getaddrinfo`` call wedged the loop while glibc
1202-
retried against zero nameservers.
1196+
and with a 2-second per-host budget. Resolving inline wedges the
1197+
whole event loop when /etc/resolv.conf is misconfigured: glibc
1198+
retries against zero nameservers and every endpoint touching
1199+
``_auto_reload_xray`` stalls with it.
12031200
12041201
DNS-broken or host-unreachable cases are absorbed silently
12051202
(worst case: loop prevention doesn't apply until the host

backend/app/api/user_agents.py

Lines changed: 32 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,8 @@
11
"""User-Agent template CRUD + JSON export/import.
22
3-
A `UserAgentTemplate` is the client fingerprint a subscription fetch
4-
presents: the `User-Agent` string plus any extra request headers the
5-
panel gates on. The catalogue used to be two hardcoded dicts in
6-
`api/subscriptions.py`; Alembic 018 seeds the table with exactly those
7-
presets, and everything here exists so the operator can edit, extend and
8-
move them between installs without a redeploy.
9-
10-
Header-merge order, the reason Happ's X-* bundle is still generated in
11-
code, and the validation ruleset all live in `app/core/ua_templates.py`.
12-
13-
Endpoint summary
14-
----------------
15-
* `GET /api/user-agents` — list, dropdown order, with usage counts
16-
* `POST /api/user-agents` — create
17-
* `GET /api/user-agents/export-json` — downloadable bundle
18-
* `POST /api/user-agents/import-json` — restore a bundle (`?replace=`, `?overwrite=`)
19-
* `GET /api/user-agents/{id}` — read one
20-
* `PATCH /api/user-agents/{id}` — edit; a key rename re-points its subscriptions
21-
* `DELETE /api/user-agents/{id}` — delete; 409 while subscriptions use it unless `?force=true`
3+
A template is the fingerprint a subscription fetch presents: the
4+
`User-Agent` string plus any extra headers the panel gates on. Header
5+
assembly and the validation ruleset live in `app/core/ua_templates.py`.
226
"""
237
import logging
248
from datetime import datetime, timezone
@@ -47,8 +31,7 @@
4731
EXPORT_KIND = "pitun-ua-templates-export"
4832
EXPORT_VERSION = 1
4933

50-
# Sanity ceiling on the table. The dropdown becomes unusable long before
51-
# this, and it bounds an import bundle's blast radius.
34+
# Bounds the dropdown and an import bundle's blast radius.
5235
MAX_TEMPLATES = 200
5336

5437

@@ -57,9 +40,8 @@
5740
async def _usage_counts(session: AsyncSession) -> Dict[str, int]:
5841
"""How many subscriptions reference each template key.
5942
60-
One query for the whole table, counted in Python — the subscription
61-
count is in the tens at most, and this avoids a GROUP BY that would
62-
need a raw-SQL escape hatch under SQLModel's async session.
43+
Counted in Python: subscriptions number in the tens, and a GROUP BY
44+
would need a raw-SQL escape hatch under SQLModel's async session.
6345
"""
6446
keys = (await session.exec(select(Subscription.ua))).all()
6547
counts: Dict[str, int] = {}
@@ -91,14 +73,12 @@ async def _get_or_404(session: AsyncSession, tpl_id: int) -> UserAgentTemplate:
9173

9274

9375
def _row_label(entry: Any) -> str:
94-
"""Safe identifier for an import row, for use in logs and error text.
95-
96-
Taken from the row's raw `key` because that is the only field present
97-
on a row that failed validation. Sanitised before it goes anywhere:
98-
control characters are stripped (CWE-117 log injection) and the value
99-
is truncated, since a bundle is untrusted input. Only the key is ever
100-
surfaced — never `user_agent` or `headers`, which can carry the
101-
operator's panel credentials.
76+
"""Safe identifier for an import row, for logs and error text.
77+
78+
The raw `key` is the only field present on a row that failed
79+
validation. Stripped of control characters (CWE-117) and truncated,
80+
since a bundle is untrusted. Never surfaces `user_agent` or
81+
`headers` — those can carry panel credentials.
10282
"""
10383
if not isinstance(entry, dict):
10484
return "?"
@@ -162,9 +142,7 @@ async def create_ua_template(
162142
return _to_read(row, 0)
163143

164144

165-
# Declared BEFORE `/{tpl_id}` so FastAPI matches the literal path instead
166-
# of reading "export-json" as an int path param (same trick as
167-
# routing_sets' /capacity).
145+
# Must precede `/{tpl_id}` or FastAPI reads "export-json" as an int param.
168146
@router.get("/export-json")
169147
async def export_ua_templates(session: AsyncSession = Depends(get_session)):
170148
"""Return every template as a downloadable JSON bundle."""
@@ -182,8 +160,7 @@ async def export_ua_templates(session: AsyncSession = Depends(get_session)):
182160
"key": r.key,
183161
"name": r.name,
184162
"user_agent": r.user_agent,
185-
# Sanitised, not raw: an exported bundle must be
186-
# re-importable, and import re-validates every header.
163+
# Sanitised so the bundle survives import re-validation.
187164
"headers": sanitize_headers(r.headers),
188165
"description": r.description,
189166
"order": r.order,
@@ -222,18 +199,12 @@ async def import_ua_templates(
222199
),
223200
session: AsyncSession = Depends(get_session),
224201
):
225-
"""Restore templates from a previously-exported bundle.
226-
227-
Three behaviours, in increasing destructiveness:
228-
229-
* default — additive; a key that already exists is **skipped**.
230-
* `overwrite=true` — additive, but a matching key is **updated** in
231-
place. Keeps its id, so subscriptions pointing at it are unaffected.
232-
* `replace=true` — wipe the table first, then insert. The only way to
233-
drop templates that aren't in the bundle.
202+
"""Restore templates from an exported bundle.
234203
235-
Rows are validated individually: one bad entry lands in `errors` and
236-
the rest still import.
204+
Additive by default (existing key skipped); `overwrite` updates a
205+
match in place, keeping its id so subscriptions stay attached;
206+
`replace` wipes first. Rows validate individually — one bad entry
207+
lands in `errors` and the rest still import.
237208
"""
238209
if not isinstance(payload, dict):
239210
raise HTTPException(400, "Invalid bundle: expected a JSON object at the top level")
@@ -261,21 +232,18 @@ async def import_ua_templates(
261232
await session.delete(row)
262233
await session.flush()
263234

264-
# Track keys seen in this bundle so a bundle containing the same key
265-
# twice reports a clean per-row error instead of tripping the UNIQUE
266-
# constraint and rolling back the whole request.
235+
# A key twice in one bundle should be a per-row error, not a UNIQUE
236+
# violation that rolls back the whole request.
267237
seen_keys: set[str] = set()
268238

269239
for entry in entries:
270-
# Label the row from its RAW key before validating, so a row that
271-
# fails validation is still identifiable in `errors` — deriving it
272-
# from the validated object would leave every rejected row as "?".
240+
# Label from the RAW key: deriving it from the validated object
241+
# would leave every rejected row as "?".
273242
row_label = _row_label(entry)
274243
try:
275244
if not isinstance(entry, dict):
276245
raise ValueError("entry is not an object")
277-
# Drop unknown fields (forward-compat: a newer export with
278-
# extra columns still imports into an older PiTun).
246+
# Forward-compat: a newer export with extra columns still imports.
279247
allowed = set(UserAgentTemplateCreate.model_fields.keys())
280248
clean = {k: v for k, v in entry.items() if k in allowed}
281249
validated = UserAgentTemplateCreate(**clean)
@@ -319,12 +287,9 @@ async def import_ua_templates(
319287
await session.flush()
320288
imported += 1
321289
except Exception as exc: # noqa: BLE001 — surface per-row errors
322-
# Log the exception *type* only, never the value: a bundle row
323-
# can carry operator secrets (panel API keys live in the
324-
# headers object) and a pydantic ValidationError message
325-
# echoes the offending input verbatim. Same envelope as nodes'
326-
# import-json (CWE-209/532/117). `row_label` is already
327-
# sanitised by `_row_label`.
290+
# Type only, never the value: a ValidationError echoes its
291+
# input and a row's headers can hold a panel API key
292+
# (CWE-209/532/117).
328293
logger.warning(
329294
"UA template import row failed: key=%r err_type=%s",
330295
row_label, type(exc).__name__,
@@ -354,12 +319,12 @@ async def update_ua_template(
354319
body: UserAgentTemplateUpdate,
355320
session: AsyncSession = Depends(get_session),
356321
):
357-
"""Edit a template, including the built-in ones.
322+
"""Edit a template, built-ins included.
358323
359-
Renaming the `key` would orphan every subscription pointing at the
360-
old value — those would silently fall back to the built-in UA map and
361-
start presenting a different fingerprint. So we re-point them in the
362-
same transaction instead.
324+
Renaming the `key` would orphan every subscription pointing at the old
325+
value — they would fall back to the built-in map and start presenting
326+
a different fingerprint — so they are re-pointed in the same
327+
transaction.
363328
"""
364329
row = await _get_or_404(session, tpl_id)
365330
patch = body.model_dump(exclude_unset=True)

backend/app/core/config_gen.py

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -695,10 +695,9 @@ def _format_rule_addr(host: str, rtype: str) -> str:
695695
# captures the DNS resolver's own connection to 8.8.8.8:53,
696696
# tunnelling it through the active node. If the node is flaky or
697697
# under handshake-failure, the whole DNS layer dies — and with
698-
# broken DNS, nothing else resolves either. Observed in the wild on
699-
# the 1256-node burn-in (192.168.1.4): user added "Port Range
700-
# 0-65535 → proxy", node had a transient handshake failure, the
701-
# whole API and proxy locked up.
698+
# broken DNS, nothing else resolves either. A catch-all proxy rule
699+
# plus one transient handshake failure is enough to lock up both
700+
# the API and the proxy.
702701
#
703702
# The `outboundTag` field on a DNS server config is exactly the
704703
# escape hatch — xray short-circuits routing for THAT server's
@@ -1091,9 +1090,7 @@ def generate_config(
10911090
# `port: 0-65535 → proxy`, `geoip:!ru → proxy`) intercepts
10921091
# the DNS dial too and tunnels it through the active VPN
10931092
# node — if that node has any handshake issue, DNS dies and
1094-
# nothing else can resolve. Observed in the wild on
1095-
# 192.168.1.4 during 1.3.4 burn-in (1256-node subscription
1096-
# + Port Range 0-65535 → proxy rule). Pair with the
1093+
# nothing else can resolve. Pair with the
10971094
# `outboundTag: direct` per-DNS-server pin in `_build_dns_section`
10981095
# — that one wins for any internal xray query that goes
10991096
# through the routing engine BEFORE hitting the outbound.
@@ -1155,8 +1152,8 @@ def generate_config(
11551152
# so a user `port: 0-65535 → proxy` (or `geoip:!ru → proxy`) rule
11561153
# would otherwise capture the DNS dial and tunnel it through the
11571154
# active VPN node, breaking the resolver when that node has any
1158-
# handshake issue. Burn-in on 192.168.1.4 nailed the exact case:
1159-
# add the catch-all rule → kill DNS → kill everything.
1155+
# handshake issue: add the catch-all rule → kill DNS → kill
1156+
# everything.
11601157
#
11611158
# Why we don't pin via dns-out's `proxySettings.tag`: that field
11621159
# chains outbounds (dns-out's resolved data flows through direct)

backend/app/core/naive_manager.py

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -228,13 +228,12 @@ def _run_container_sync(self, node: Node) -> None:
228228
type=docker.types.LogConfig.types.JSON,
229229
config={"max-size": "10m", "max-file": "3"},
230230
),
231-
# 96 MB (was 64 MB through v1.2.5). Observed `exit=137`
232-
# SIGKILL on cold start at 192.168.1.3 — Caddy's TLS
233-
# session-ticket cache + naive's QUIC connection pool
234-
# spike RSS briefly above 64 MB on a freshly provisioned
235-
# container. 96 MB gives headroom without significant
236-
# memory pressure on a Pi 4 (typical sidecar steady-state
237-
# is ~30-40 MB). Combined with the supervisor's
231+
# 96 MB. At 64 MB a cold start could `exit=137` (OOM
232+
# SIGKILL): Caddy's TLS session-ticket cache plus naive's
233+
# QUIC connection pool spike RSS briefly on a freshly
234+
# provisioned container. 96 MB gives headroom without
235+
# significant memory pressure on a Pi 4 (typical sidecar
236+
# steady-state is ~30-40 MB). Combined with the supervisor's
238237
# exponential backoff (v1.2.6), tight OOM-restart loops
239238
# should no longer be reachable.
240239
mem_limit="96m",

0 commit comments

Comments
 (0)