Skip to content

feat: Add final_mask_settings to hosts#451

Open
ImMohammad20000 wants to merge 6 commits intodevfrom
final-mask-builder
Open

feat: Add final_mask_settings to hosts#451
ImMohammad20000 wants to merge 6 commits intodevfrom
final-mask-builder

Conversation

@ImMohammad20000
Copy link
Copy Markdown
Contributor

@ImMohammad20000 ImMohammad20000 commented May 7, 2026

Summary by CodeRabbit

  • New Features
    • Final mask settings now available for host configuration, enabling enhanced control over inbound traffic behavior with structured and validated configuration options.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 7, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3b28d5ad-28d8-4498-a282-463da9a465ac

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • ✅ Review completed - (🔄 Check again to review again)
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch final-mask-builder

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
app/subscription/xray.py (1)

610-617: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Pass finalmask as a dict here too — currently a FinalMask Pydantic instance can leak into json.dumps.

_build_shadowsocks (lines 470–475) correctly converts inbound.finalmask to a dict via model_dump() before calling _stream_setting_config. This branch passes inbound.finalmask through unchanged, so when the host has a structured FinalMask value (which app/models/subscription.py now allows), it ends up assigned to stream_settings["finalmask"] as a Pydantic model. The eventual json.dumps(self.config, ..., cls=UUIDEncoder) in render() will raise TypeError because UUIDEncoder does not know how to serialize a FinalMask instance.

Apply the same conversion as _build_shadowsocks, ideally factored into a small helper to keep the two sites in sync:

🐛 Proposed fix
+    `@staticmethod`
+    def _serialize_finalmask(finalmask):
+        if finalmask is None:
+            return None
+        if isinstance(finalmask, FinalMask):
+            return finalmask.model_dump()
+        return finalmask
+
@@
-        outbound["streamSettings"] = self._stream_setting_config(
-            network=network,
-            security=security,
-            network_setting=network_setting,
-            tls_settings=tls_settings,
-            sockopt=sockopt,
-            finalmask=inbound.finalmask,
-        )
+        outbound["streamSettings"] = self._stream_setting_config(
+            network=network,
+            security=security,
+            network_setting=network_setting,
+            tls_settings=tls_settings,
+            sockopt=sockopt,
+            finalmask=self._serialize_finalmask(inbound.finalmask),
+        )

And reuse the helper in _build_shadowsocks:

-        if inbound.finalmask is not None:
-            if isinstance(inbound.finalmask, FinalMask):
-                finalmask = inbound.finalmask.model_dump()
-            else:
-                finalmask = inbound.finalmask
-            outbound["streamSettings"] = self._stream_setting_config(network=inbound.network, finalmask=finalmask)
+        finalmask = self._serialize_finalmask(inbound.finalmask)
+        if finalmask is not None:
+            outbound["streamSettings"] = self._stream_setting_config(network=inbound.network, finalmask=finalmask)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/subscription/xray.py` around lines 610 - 617, The streamSettings branch
is passing inbound.finalmask (a Pydantic FinalMask instance) directly into
_stream_setting_config which can leak a model into config JSON; add a small
helper (e.g., _finalmask_to_dict or reuse a wrapper) that calls
inbound.finalmask.model_dump() if the value is a Pydantic model (or returns it
unchanged if already a dict), then call that helper when building streamSettings
(the same way _build_shadowsocks currently converts inbound.finalmask) so
_stream_setting_config always receives a plain dict and no FinalMask instances
reach json.dumps in render().
app/subscription/links.py (1)

7-16: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Missing FinalMask import — _apply_finalmask will raise NameError and CI is already failing.

Line 170 references FinalMask, but it is never imported in this file. Ruff (F821) and the Code Format Checker pipeline both fail on this. At runtime, any call to _apply_finalmask with a non-empty inbound.finalmask will raise NameError: name 'FinalMask' is not defined.

🐛 Proposed fix
 from app.models.subscription import (
     GRPCTransportConfig,
     KCPTransportConfig,
     QUICTransportConfig,
     SubscriptionInboundData,
     TCPTransportConfig,
     TLSConfig,
     WebSocketTransportConfig,
     XHTTPTransportConfig,
 )
+from app.models.host import FinalMask

Also worth factoring the isinstance(..., FinalMask) branch with the identical one in app/subscription/xray.py::_build_shadowsocks into a small shared helper to avoid drift.

Also applies to: 167-174

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/subscription/links.py` around lines 7 - 16, Import FinalMask into
app/subscription/links.py so that references inside _apply_finalmask resolve;
update the top import block to include FinalMask alongside the other models
(e.g., GRPCTransportConfig, KCPTransportConfig, ...). Also consider extracting
the isinstance(..., FinalMask) handling into a small shared helper used by both
app/subscription/links.py::_apply_finalmask and
app/subscription/xray.py::_build_shadowsocks to avoid duplicated logic and keep
behavior consistent.
🧹 Nitpick comments (3)
app/core/hosts.py (1)

175-175: 💤 Low value

LGTM, with a small nit on the truthiness check.

host.final_mask_settings is now a Pydantic FinalMask model, which is always truthy regardless of whether its fields are populated. If the intent is to fall back to inbound_config["finalmask"] only when the host explicitly hasn't set one, prefer an explicit is not None check (consistent with how host.subscription_templates is handled elsewhere in this function):

♻️ Optional
-    final_mask_settings = host.final_mask_settings if host.final_mask_settings else inbound_config.get("finalmask")
+    final_mask_settings = (
+        host.final_mask_settings if host.final_mask_settings is not None else inbound_config.get("finalmask")
+    )
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/core/hosts.py` at line 175, The current fallback uses a truthiness check
on host.final_mask_settings which is a Pydantic FinalMask model and always
truthy; change the condition to an explicit None check so we only use
inbound_config.get("finalmask") when the host has not set one (i.e., replace the
truthiness check with "is not None" when assigning final_mask_settings in the
same block that handles host.subscription_templates to mirror that logic).
app/models/host.py (2)

37-52: 💤 Low value

Nit: the delay → interval remap interacts with serialization_alias="delay".

interval keeps serialization_alias="delay", so:

  • Input: both {"interval": "..."} and the legacy {"delay": "..."} validate (good).
  • Output via model_dump() (default by_alias=False) emits "interval"; only model_dump(by_alias=True) emits "delay".

In app/subscription/xray.py::_build_shadowsocks and _build_outbound, the FinalMask is dumped via plain model_dump(), so the output key will be "interval". Please confirm the consumer (xray-core fragment) actually expects "interval" here (it does for stream-level fragment, but please double-check the finalMask schema), and that the "delay" alias is only intended for an external API/UI layer. If both representations matter, prefer validation_alias=AliasChoices("delay", "interval") so naming intent is explicit.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/models/host.py` around lines 37 - 52, The delay→interval remap in
XrayFragmentSettings uses serialization_alias="delay" on the interval Field
while the before-validator delay_to_interval accepts input "delay", but code
that dumps FinalMask via model_dump() in
app/subscription/xray.py::_build_shadowsocks and _build_outbound will emit
"interval" (not "delay"); either ensure the xray-core finalMask consumer accepts
"interval" or make the model explicitly support both names for
validation/serialization: replace serialization_alias with
validation_alias=AliasChoices("delay","interval") on the interval Field (or
remove the alias and always call model_dump(by_alias=True) where FinalMask is
emitted) and keep the delay_to_interval validator on XrayFragmentSettings to
preserve legacy input handling so both input and output names are consistent
with the consumer.

191-202: 💤 Low value

Minor: redundant Enum coercion since use_enum_values=True.

With use_enum_values=True on FinalMaskBaseModel, self.type is already the string value (e.g. "header-custom"), so wrapping the dict lookup key as FinalMaskTcpType(self.type) is round-trip work. You can just key the dict by the string values, or drop use_enum_values=True. Same applies to FinalMaskUdpLayer.parse_settings.

Also note: FinalMaskTcpType only has three members and all three are present in the mapping today, so settings_model is always non-None here — but the symmetrical UDP version does guard with if settings_model is not None. Adding the same guard for TCP would protect against future enum additions.

♻️ Optional
-        settings_model = {
-            FinalMaskTcpType.header_custom: FinalMaskTcpHeaderCustomSettings,
-            FinalMaskTcpType.fragment: XrayFragmentSettings,
-            FinalMaskTcpType.sudoku: FinalMaskSudokuSettings,
-        }.get(FinalMaskTcpType(self.type))
-        self.settings = settings_model.model_validate(self.settings)
-        return self
+        settings_model = {
+            FinalMaskTcpType.header_custom.value: FinalMaskTcpHeaderCustomSettings,
+            FinalMaskTcpType.fragment.value: XrayFragmentSettings,
+            FinalMaskTcpType.sudoku.value: FinalMaskSudokuSettings,
+        }.get(self.type)
+        if settings_model is not None:
+            self.settings = settings_model.model_validate(self.settings)
+        return self
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/models/host.py` around lines 191 - 202, In parse_settings on the TCP
model, avoid the redundant Enum coercion by keying the settings_model mapping
with the enum's string values (e.g. use the literal keys matching
use_enum_values like "header-custom", "fragment", "sudoku") instead of wrapping
self.type with FinalMaskTcpType(self.type); also mirror the UDP guard by
checking if settings_model is not None before calling
settings_model.model_validate so future enum members don't cause an exception
(refer to parse_settings, FinalMaskTcpType members, and the pattern used in
FinalMaskUdpLayer.parse_settings).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@app/db/migrations/versions/f976bfcf4738_add_final_mask_settings_to_hosts_table.py`:
- Around line 1-16: The migration header docstring's "Revises:" value doesn't
match the down_revision variable; update the docstring line `Revises:
b7d9e1a2c3f4` to `Revises: af2d644dda44` so it matches the `down_revision =
'af2d644dda44'` (identify the migration by revision = 'f976bfcf4738') — only
change the docstring header to keep history consistent.

In `@app/models/host.py`:
- Around line 173-184: The union types FinalMaskTcpSettings and
FinalMaskUdpSettings are being prematurely coerced by Pydantic v2 smart-mode
union resolution before your parse_settings validator can re-dispatch by type,
so change parse_settings to run with mode="before" (re-dispatch on the raw dict
using the "type" field) for both the FinalMaskUdpLayer and FinalMaskTcpLayer
validators to select the correct settings_model before any union validation;
also update the dispatch dict used in parse_settings to include explicit
handlers for the six missing FinalMaskUdpType values (header_dtls, header_srtp,
header_utp, header_wechat, header_wireguard, mkcp_original) or add a clear
fallback that maps them to dict[str, Any] so settings are validated/kept
correctly instead of relying on union coercion.

---

Outside diff comments:
In `@app/subscription/links.py`:
- Around line 7-16: Import FinalMask into app/subscription/links.py so that
references inside _apply_finalmask resolve; update the top import block to
include FinalMask alongside the other models (e.g., GRPCTransportConfig,
KCPTransportConfig, ...). Also consider extracting the isinstance(...,
FinalMask) handling into a small shared helper used by both
app/subscription/links.py::_apply_finalmask and
app/subscription/xray.py::_build_shadowsocks to avoid duplicated logic and keep
behavior consistent.

In `@app/subscription/xray.py`:
- Around line 610-617: The streamSettings branch is passing inbound.finalmask (a
Pydantic FinalMask instance) directly into _stream_setting_config which can leak
a model into config JSON; add a small helper (e.g., _finalmask_to_dict or reuse
a wrapper) that calls inbound.finalmask.model_dump() if the value is a Pydantic
model (or returns it unchanged if already a dict), then call that helper when
building streamSettings (the same way _build_shadowsocks currently converts
inbound.finalmask) so _stream_setting_config always receives a plain dict and no
FinalMask instances reach json.dumps in render().

---

Nitpick comments:
In `@app/core/hosts.py`:
- Line 175: The current fallback uses a truthiness check on
host.final_mask_settings which is a Pydantic FinalMask model and always truthy;
change the condition to an explicit None check so we only use
inbound_config.get("finalmask") when the host has not set one (i.e., replace the
truthiness check with "is not None" when assigning final_mask_settings in the
same block that handles host.subscription_templates to mirror that logic).

In `@app/models/host.py`:
- Around line 37-52: The delay→interval remap in XrayFragmentSettings uses
serialization_alias="delay" on the interval Field while the before-validator
delay_to_interval accepts input "delay", but code that dumps FinalMask via
model_dump() in app/subscription/xray.py::_build_shadowsocks and _build_outbound
will emit "interval" (not "delay"); either ensure the xray-core finalMask
consumer accepts "interval" or make the model explicitly support both names for
validation/serialization: replace serialization_alias with
validation_alias=AliasChoices("delay","interval") on the interval Field (or
remove the alias and always call model_dump(by_alias=True) where FinalMask is
emitted) and keep the delay_to_interval validator on XrayFragmentSettings to
preserve legacy input handling so both input and output names are consistent
with the consumer.
- Around line 191-202: In parse_settings on the TCP model, avoid the redundant
Enum coercion by keying the settings_model mapping with the enum's string values
(e.g. use the literal keys matching use_enum_values like "header-custom",
"fragment", "sudoku") instead of wrapping self.type with
FinalMaskTcpType(self.type); also mirror the UDP guard by checking if
settings_model is not None before calling settings_model.model_validate so
future enum members don't cause an exception (refer to parse_settings,
FinalMaskTcpType members, and the pattern used in
FinalMaskUdpLayer.parse_settings).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 69646fb6-43f0-40d7-8eb1-6adeee3ad1b5

📥 Commits

Reviewing files that changed from the base of the PR and between 1dce901 and 9cb6db0.

📒 Files selected for processing (7)
  • app/core/hosts.py
  • app/db/migrations/versions/f976bfcf4738_add_final_mask_settings_to_hosts_table.py
  • app/db/models.py
  • app/models/host.py
  • app/models/subscription.py
  • app/subscription/links.py
  • app/subscription/xray.py

Comment on lines +1 to +16
"""add final_mask_settings to hosts table

Revision ID: f976bfcf4738
Revises: b7d9e1a2c3f4
Create Date: 2026-05-02 13:46:21.008567

"""
from alembic import op
import sqlalchemy as sa


# revision identifiers, used by Alembic.
revision = 'f976bfcf4738'
down_revision = 'af2d644dda44'
branch_labels = None
depends_on = None
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check that af2d644dda44 exists and is the (single) head before this migration is added.
fd -t f -e py . app/db/migrations/versions | xargs rg -nP "^(revision|down_revision)\s*="

Repository: PasarGuard/panel

Length of output: 25777


Update docstring Revises: to match down_revision.

The header docstring says Revises: b7d9e1a2c3f4, but down_revision = 'af2d644dda44'. Alembic only uses the variable, so the migration runs correctly, but the docstring mismatch is confusing for anyone reviewing the migration history.

📝 Proposed fix
-"""add final_mask_settings to hosts table
-
-Revision ID: f976bfcf4738
-Revises: b7d9e1a2c3f4
-Create Date: 2026-05-02 13:46:21.008567
-
-"""
+"""add final_mask_settings to hosts table
+
+Revision ID: f976bfcf4738
+Revises: af2d644dda44
+Create Date: 2026-05-02 13:46:21.008567
+
+"""
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"""add final_mask_settings to hosts table
Revision ID: f976bfcf4738
Revises: b7d9e1a2c3f4
Create Date: 2026-05-02 13:46:21.008567
"""
from alembic import op
import sqlalchemy as sa
# revision identifiers, used by Alembic.
revision = 'f976bfcf4738'
down_revision = 'af2d644dda44'
branch_labels = None
depends_on = None
"""add final_mask_settings to hosts table
Revision ID: f976bfcf4738
Revises: af2d644dda44
Create Date: 2026-05-02 13:46:21.008567
"""
from alembic import op
import sqlalchemy as sa
# revision identifiers, used by Alembic.
revision = 'f976bfcf4738'
down_revision = 'af2d644dda44'
branch_labels = None
depends_on = None
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@app/db/migrations/versions/f976bfcf4738_add_final_mask_settings_to_hosts_table.py`
around lines 1 - 16, The migration header docstring's "Revises:" value doesn't
match the down_revision variable; update the docstring line `Revises:
b7d9e1a2c3f4` to `Revises: af2d644dda44` so it matches the `down_revision =
'af2d644dda44'` (identify the migration by revision = 'f976bfcf4738') — only
change the docstring header to keep history consistent.

Comment thread app/models/host.py
Comment on lines +173 to +184
FinalMaskTcpSettings = (
FinalMaskTcpHeaderCustomSettings | XrayFragmentSettings | FinalMaskSudokuSettings | dict[str, Any]
)
FinalMaskUdpSettings = (
FinalMaskUdpHeaderCustomSettings
| FinalMaskPasswordSettings
| FinalMaskSudokuSettings
| FinalMaskDomainSettings
| FinalMaskXicmpSettings
| FinalMaskNoiseSettings
| dict[str, Any]
)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🌐 Web query:

Pydantic v2 smart union resolution order with multiple models that have extra=allow

💡 Result:

In Pydantic v2, non-discriminated Union validation defaults to union_mode='smart' [1][2]. In smart mode, Pydantic evaluates all union members (attempting them left-to-right) and then picks the “best” match using these metrics: 1) It scores each successful member match by the number of valid fields set (for models/dataclasses/typed dicts; nested field counts bubble up) [1][2]. 2) After all members are evaluated, the member with the highest “valid fields set” count wins [1][2]. 3) If tied on “valid fields set”, it uses an “exactness” score as a tiebreaker [1][2]. So, “resolution order” in smart mode is not purely left-to-right; left-to-right is only the evaluation order, while the final selection is driven primarily by fields-set count (then exactness) [1][2]. What changes when union members use extra='allow'? - There is a known interaction/shortcoming in smart union matching when union members have no required fields and use extra='allow' (or extra='ignore'): models can receive points from extra fields, leading to unintuitive member selection [3]. - Pydantic describes that it fixed the extra='ignore' case via pydantic-core, but the extra='allow' case required further handling / a follow-up PR discussion [3]. - A pydantic-core change discussed improving union validation decisions based on extra behavior exists (context around tie-breaking and adding an extra-related metric) [4]. Practical takeaway - If you have multiple models in a Union that might both successfully validate the same input (especially when they have overlapping structure and allow extra fields), smart mode will prefer the member that results in the highest count of “valid fields set” (and only then exactness) [1][2], but extra='allow' can skew that selection in ways that were considered unintuitive and were actively worked on [3][4]. - For fully predictable selection when you have a “tag” field, use discriminated unions (Field(discriminator=...)), which validate only one member instead of doing best-match scoring [5]. If you share a minimal repro (Union type + the two/three models + the exact input dict), I can tell you which member smart mode will pick given your field definitions and extra='allow' behavior, and whether you can/should switch to discriminators or left_to_right to lock it down.

Citations:


🏁 Script executed:

cd app/models && cat -n host.py | sed -n '173,184p'

Repository: PasarGuard/panel

Length of output: 509


🏁 Script executed:

cd app/models && cat -n host.py | sed -n '37,90p'

Repository: PasarGuard/panel

Length of output: 2311


🏁 Script executed:

cd app/models && cat -n host.py | sed -n '187,226p'

Repository: PasarGuard/panel

Length of output: 1982


🏁 Script executed:

cd app/models && cat -n host.py | sed -n '1,35p'

Repository: PasarGuard/panel

Length of output: 1031


🏁 Script executed:

cd app/models && rg "class FinalMaskUdpType" -A 15

Repository: PasarGuard/panel

Length of output: 646


🏁 Script executed:

cd app/models && rg "class FinalMask(TcpHeaderCustomSettings|UdpHeaderCustomSettings|PasswordSettings|DomainSettings|XicmpSettings|NoiseSettings|SudokuSettings)" -A 5

Repository: PasarGuard/panel

Length of output: 1776


🏁 Script executed:

cd app/models && fd . --type f -name "host.py" -exec wc -l {} \;

Repository: PasarGuard/panel

Length of output: 291


🏁 Script executed:

cd app/models && sed -n '90,172p' host.py | cat -n

Repository: PasarGuard/panel

Length of output: 3896


🏁 Script executed:

cd app/models && rg "header_dtls|header_srtp|header_utp|header_wechat|header_wireguard|mkcp_original" --type py

Repository: PasarGuard/panel

Length of output: 316


🏁 Script executed:

cd app/models && python3 << 'EOF'
# Analyze field requirements for union members
import re

fields = {
    "FinalMaskTcpHeaderCustomSettings": {"clients": "optional", "servers": "optional", "errors": "optional"},
    "XrayFragmentSettings": {"packets": "required", "length": "required", "interval": "required", "max_split": "optional"},
    "FinalMaskSudokuSettings": {"password": "optional", "ascii": "optional", "custom_table": "optional", "custom_tables": "optional", "padding_min": "optional", "padding_max": "optional"},
    "FinalMaskUdpHeaderCustomSettings": {"client": "optional", "server": "optional"},
    "FinalMaskPasswordSettings": {"password": "optional"},
    "FinalMaskDomainSettings": {"domain": "optional"},
    "FinalMaskXicmpSettings": {"listen_ip": "optional", "id": "optional"},
    "FinalMaskNoiseSettings": {"reset": "optional", "noise": "optional"},
}

print("Union members field analysis:")
for model, model_fields in fields.items():
    required = [f for f, req in model_fields.items() if req == "required"]
    optional = [f for f, req in model_fields.items() if req == "optional"]
    print(f"\n{model}:")
    print(f"  Required: {required if required else 'None'}")
    print(f"  Optional: {optional}")
    print(f"  Has required fields: {bool(required)}")
EOF

Repository: PasarGuard/panel

Length of output: 1021


🏁 Script executed:

cd app && rg "FinalMaskLayer\|parse_settings" --type py -A 2 -B 2

Repository: PasarGuard/panel

Length of output: 42


🏁 Script executed:

cd app && grep -r "header-dtls\|header-srtp\|header-utp\|header-wechat\|header-wireguard\|mkcp-original" --include="*.py" --include="*.json" | head -20

Repository: PasarGuard/panel

Length of output: 358


🏁 Script executed:

cd app && rg "FinalMaskUdpLayer|FinalMaskTcpLayer" -B 5 -A 10 --type py | head -100

Repository: PasarGuard/panel

Length of output: 2404


🏁 Script executed:

cd app && find . -type f \( -name "*.py" -o -name "*.json" \) -exec grep -l "FinalMask" {} \; | head -10

Repository: PasarGuard/panel

Length of output: 149


🏁 Script executed:

cd app && rg "\.get\(FinalMask" -A 3 -B 3 --type py

Repository: PasarGuard/panel

Length of output: 964


FinalMask*Settings unions are fragile due to union validation occurring before type-aware re-dispatch.

FinalMaskUdpSettings and FinalMaskTcpSettings are unions of models that all inherit extra="allow". During initial validation, Pydantic v2 uses smart-mode union resolution, which ranks candidates by field-set count rather than evaluation order; since all UDP members are entirely optional and allow extra fields, the union cannot reliably distinguish between FinalMaskUdpHeaderCustomSettings, FinalMaskPasswordSettings, FinalMaskDomainSettings, etc. The result is coerced into whichever union member scores highest by Pydantic's ranking, regardless of the actual type field.

Because parse_settings runs in mode="after", this union coercion is already complete by the time the validator checks if not isinstance(self.settings, dict): return self. If the union picked the wrong model, that early-exit suppresses any re-dispatch.

Additionally, six FinalMaskUdpType enum values — header_dtls, header_srtp, header_utp, header_wechat, header_wireguard, mkcp_original — have no entry in the dispatch dict. For these types, settings_model remains None and parse_settings skips validation entirely, leaving settings as whatever Pydantic's union coercion produced.

A more robust approach is to use mode="before" to re-dispatch based on the type field before union coercion happens:

♻️ Suggested approach
 class FinalMaskUdpLayer(FinalMaskBaseModel):
     type: FinalMaskUdpType
-    settings: FinalMaskUdpSettings = Field(default_factory=dict)
+    settings: dict[str, Any] | FinalMaskUdpSettings = Field(default_factory=dict)
 
-    `@model_validator`(mode="after")
-    def parse_settings(self):
-        if not isinstance(self.settings, dict):
-            return self
+    `@model_validator`(mode="before")
+    `@classmethod`
+    def parse_settings(cls, values):
+        if not isinstance(values, dict):
+            return values
+        settings = values.get("settings")
+        if not isinstance(settings, dict):
+            return values
+        type_value = values.get("type")
+        try:
+            udp_type = FinalMaskUdpType(type_value)
+        except ValueError:
+            return values
+        settings_model = {
+            FinalMaskUdpType.header_custom: FinalMaskUdpHeaderCustomSettings,
+            FinalMaskUdpType.header_dns: FinalMaskDomainSettings,
+            FinalMaskUdpType.mkcp_aes128gcm: FinalMaskPasswordSettings,
+            FinalMaskUdpType.noise: FinalMaskNoiseSettings,
+            FinalMaskUdpType.salamander: FinalMaskPasswordSettings,
+            FinalMaskUdpType.sudoku: FinalMaskSudokuSettings,
+            FinalMaskUdpType.xdns: FinalMaskDomainSettings,
+            FinalMaskUdpType.xicmp: FinalMaskXicmpSettings,
+        }.get(udp_type)
+        if settings_model is not None:
+            values["settings"] = settings_model.model_validate(settings)
+        return values

Apply the analogous mode="before" rewrite to FinalMaskTcpLayer. Also consider adding explicit handlers (or a fallback dict path) for the unmapped UDP types.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/models/host.py` around lines 173 - 184, The union types
FinalMaskTcpSettings and FinalMaskUdpSettings are being prematurely coerced by
Pydantic v2 smart-mode union resolution before your parse_settings validator can
re-dispatch by type, so change parse_settings to run with mode="before"
(re-dispatch on the raw dict using the "type" field) for both the
FinalMaskUdpLayer and FinalMaskTcpLayer validators to select the correct
settings_model before any union validation; also update the dispatch dict used
in parse_settings to include explicit handlers for the six missing
FinalMaskUdpType values (header_dtls, header_srtp, header_utp, header_wechat,
header_wireguard, mkcp_original) or add a clear fallback that maps them to
dict[str, Any] so settings are validated/kept correctly instead of relying on
union coercion.

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