Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions app/sep/apps/atw/batch.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,16 @@
NonEmptyStr,
UTCDatetime,
)
from app.sep.apps.field_names import (
EXECUTOR_HOST_FIELD_NAME,
EXTRA_ARGS_FIELD_NAME,
SCRIPT_PREVIEW_FIELD_NAME,
SUDO_FIELD_NAME,
)
from app.sep.apps.framework.schema import (
AnyField,
BoolField,
EXECUTOR_HOST_FIELD_NAME,
HostField,
SCRIPT_PREVIEW_FIELD_NAME,
SUDO_FIELD_NAME,
)
from app.sep.apps.framework.script_helpers import execute_script
from app.sep.apps.framework.script_source import (
Expand All @@ -52,7 +55,6 @@
ScriptExecutionResponse,
)
from app.sep.apps.labels import EXECUTION_HOST_LABEL
from app.sep.snippets.models.constants import EXTRA_ARGS_FIELD_NAME
from app.sep.snippets.script_source import snippet_source, SnippetScript
from app.tasks.models import TaskHistoryStatusEnum

Expand Down
28 changes: 27 additions & 1 deletion app/sep/apps/dipper/deps.py
Original file line number Diff line number Diff line change
Expand Up @@ -471,10 +471,36 @@ def build_dipper_meta_from_args(
*,
sudo_default: bool = False,
) -> SnippetExecutionMeta:
"""Build shared execution metadata for legacy and JSON Dipper flows."""
"""Build shared execution metadata for legacy and JSON Dipper flows.

Both flows assemble their meta here, so the guard against invalid frontmatter
parameters lives here rather than at either entry point. A parameter the
frontmatter declared but the parser rejected -- a reserved execution field
name, say -- is dropped from the form, so executing anyway would silently run
the script without an argument its author asked for.

The refusal enumerates the parser's own messages because this app does not
surface them on the form: without them the operator would see a rejection
naming no cause anywhere in the UI.

:param service: The inventory service the collector runs against.
:param script: The collector script being dispatched.
:param script_source: The signed URL the executor downloads the script from.
:param execution_args: The validated arguments for this execution.
:param sudo_default: Whether to run with ``sudo`` when the args request nothing.
:return: The execution metadata the framework posts to the Tasks API.
:raises HTTPBadRequestException: When the script declares no runnable
interpreter, or carries invalid frontmatter parameters.
"""
interpreter = script.execution_interpreter
if interpreter is None:
raise HTTPBadRequestException(detail="No interpreter configured for script")
if not script.can_execute:
reasons = "; ".join(script.validated_parameters.errors)
raise HTTPBadRequestException(
detail=f"Script {script.filename!r} has invalid frontmatter parameters: "
f"{reasons}"
)
return build_execution_meta(
script,
execution_args,
Expand Down
8 changes: 5 additions & 3 deletions app/sep/apps/dipper/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@
from urllib.parse import urlencode

from app.inventory.models import ServiceTypeEnum
from app.sep.apps.field_names import (
EXECUTOR_HOST_FIELD_NAME,
SCRIPT_PREVIEW_FIELD_NAME,
SUDO_FIELD_NAME,
)
from app.sep.apps.framework.schema import (
AnyField,
AppSchema,
Expand All @@ -32,14 +37,11 @@
DetailSection,
DetailView,
EXECUTION_HOST_LABEL,
EXECUTOR_HOST_FIELD_NAME,
FormSection,
HostField,
ListView,
SCRIPT_PREVIEW_FIELD_NAME,
ScriptPreviewField,
ServiceField,
SUDO_FIELD_NAME,
)
from app.sep.snippets.config import SnippetSudoOption
from app.sep.snippets.models.snippet import BaseSnippet
Expand Down
68 changes: 68 additions & 0 deletions app/sep/apps/field_names.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
# Copyright (C) 2026 Percona LLC
#
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU Affero General Public License as published by
# the Free Software Foundation, either version 3 of the License, or
# (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU Affero General Public License for more details.
#
# You should have received a copy of the GNU Affero General Public License
# along with this program. If not, see <https://www.gnu.org/licenses/>.

"""Name the execution fields a script app's form can synthesize.

Together these are the union of what the script apps append to a script's
frontmatter parameters when building an execution form -- each app synthesizes
its own subset, and some of those are conditional on the script's configuration.
A consumer that merges or strips them needs the same spelling the producer used,
whichever app produced them. :data:`RESERVED_EXECUTION_FIELD_NAMES` closes the loop
back at the other producer of form fields, the script author: a frontmatter
parameter declaring one of these names is rejected at parse time by
:meth:`app.sep.snippets.models.meta.SnippetMetaParameter._reject_reserved_name`, so
a synthesized field can never collide with an author-declared one.

Lives alongside (not inside) :mod:`app.sep.apps.framework` because both ends of an
existing one-way dependency need these spellings:
:mod:`app.sep.apps.framework.script_helpers` imports
:mod:`app.sep.snippets.models.snippet`, so nothing under ``app.sep.snippets.models``
can import back from ``framework``. This module imports nothing, so the form
builders under ``framework`` and the parameter model under ``snippets.models`` can
both depend on it without closing that loop.
"""

EXECUTOR_HOST_FIELD_NAME = "executor_host"
"""Name the synthesized execution-host field on the wire."""

SUDO_FIELD_NAME = "sudo"
"""Name the synthesized sudo toggle on the wire."""

SCRIPT_PREVIEW_FIELD_NAME = "script_preview"
"""Name the synthesized script-preview field on the wire."""

EXTRA_ARGS_FIELD_NAME = "extra_args"
"""Name the synthesized Extra Args field on the wire."""

RESERVED_EXECUTION_FIELD_NAMES = frozenset(
{
EXECUTOR_HOST_FIELD_NAME,
SUDO_FIELD_NAME,
SCRIPT_PREVIEW_FIELD_NAME,
EXTRA_ARGS_FIELD_NAME,
}
)
"""Reserve every synthesized field name against frontmatter parameter names.

Reservation is unconditional and app-wide: a name is reserved even for a script
whose app never synthesizes that particular field -- a ``sudo: never`` snippet, a
disk-backed script app with no preview endpoint -- because the alternative is a
per-app, per-configuration rule that the author of a frontmatter file cannot
evaluate from the file in front of them.

The members are the same constants the form builders synthesize from, so reserving
a fifth name is a two-line edit in this module and cannot be forgotten at the
validator.
"""
11 changes: 0 additions & 11 deletions app/sep/apps/framework/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,17 +89,6 @@
)
from app.sep.apps.labels import EXECUTION_HOST_LABEL

EXECUTOR_HOST_FIELD_NAME = "executor_host"
SUDO_FIELD_NAME = "sudo"
SCRIPT_PREVIEW_FIELD_NAME = "script_preview"
"""Name the execution fields every script app's form synthesises.

Each script app appends these to its frontmatter parameters, and a consumer that
merges or strips them needs the same spelling the producer used. They live here,
next to the field types they name, so no app package owns the vocabulary its
siblings depend on.
"""

# Dots are permitted so nested one-of branch fields can use paths such as
# ``source.source_db_id`` (see :class:`OneOfGroup`).
_FIELD_NAME_PATTERN = r"^[A-Za-z_](?:[\w.-]*\w)?$"
Expand Down
3 changes: 1 addition & 2 deletions app/sep/apps/shared/disk_script_source.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,16 +44,15 @@
HTTPNotFoundException,
HTTPUnprocessableEntityException,
)
from app.sep.apps.field_names import EXECUTOR_HOST_FIELD_NAME, SUDO_FIELD_NAME
from app.sep.apps.framework.schema import (
AppSchema,
BoolField,
Column,
EXECUTION_HOST_LABEL,
EXECUTOR_HOST_FIELD_NAME,
FormSection,
HostField,
ListView,
SUDO_FIELD_NAME,
)
from app.sep.apps.framework.script_helpers import (
build_artifact_download_url,
Expand Down
26 changes: 0 additions & 26 deletions app/sep/snippets/models/constants.py

This file was deleted.

15 changes: 11 additions & 4 deletions app/sep/snippets/models/meta.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
field_with_metadata,
loc_to_dot_sep,
)
from app.sep.apps.field_names import RESERVED_EXECUTION_FIELD_NAMES
from app.sep.snippets.forms import (
CheckboxInputElement,
DateTimeInputElement,
Expand All @@ -77,7 +78,6 @@
TextInputElement,
TextInputHTMLElement,
)
from app.sep.snippets.models.constants import EXTRA_ARGS_FIELD_NAME

ParameterType = str | int | float | bool | datetime | None

Expand Down Expand Up @@ -317,14 +317,21 @@ class SnippetMetaParameter(BaseModel):
def _reject_reserved_name(cls, value: str) -> str:
"""Reject a parameter name reserved for a synthesized execution field.

Reservation is unconditional and case-sensitive. A name in
:data:`~app.sep.apps.field_names.RESERVED_EXECUTION_FIELD_NAMES` is
rejected even when the app rendering this script never synthesises that
particular field, so an author can apply the rule to the frontmatter in
front of them without knowing which app will render it.

:param value: The candidate parameter name.
:return: ``value`` unchanged, when it is not reserved.
:raises ValueError: When the name matches a reserved synthesized field name.
"""
if value == EXTRA_ARGS_FIELD_NAME:
if value in RESERVED_EXECUTION_FIELD_NAMES:
reserved = ", ".join(sorted(RESERVED_EXECUTION_FIELD_NAMES))
raise ValueError(
f"parameter name {value!r} is reserved for the synthesized "
"Extra Args field"
f"parameter name {value!r} is reserved for a synthesized "
f"execution field; rename it (reserved names: {reserved})"
)
return value

Expand Down
11 changes: 5 additions & 6 deletions app/sep/snippets/models/snippet.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
UTCDatetime,
)
from app.core.utils.pydantic import CustomFieldMetadata
from app.sep.apps.field_names import EXTRA_ARGS_FIELD_NAME, SUDO_FIELD_NAME
from app.sep.apps.labels import EXECUTION_HOST_LABEL
from app.sep.snippets.config import (
DEFAULT_SNIPPETS_TASK,
Expand All @@ -84,7 +85,6 @@
SubmitButtonElement,
TextInputElement,
)
from app.sep.snippets.models.constants import EXTRA_ARGS_FIELD_NAME
from app.sep.snippets.models.meta import (
META_KEY_DESCRIPTION,
META_KEY_SERVICE_TYPE,
Expand Down Expand Up @@ -326,18 +326,17 @@ async def _from_path(


class BaseSnippetArgs(BaseModel):
"""Base model for validating snippet execution arguments.
"""Validate the arguments a snippet execution was submitted with.

:cvar extra_args_field: The name of the field used to store extra arguments
("extra_args").
:vartype extra_args_field: ClassVar[str]
(``extra_args``).
:cvar sudo_field: The name of the field used to store the sudo toggle (``sudo``).
:param executor_host: The hostname of the target system where the snippet will be
executed.
:type executor_host: NonEmptyStr
"""

extra_args_field: ClassVar[str] = EXTRA_ARGS_FIELD_NAME
sudo_field: ClassVar[str] = "sudo"
sudo_field: ClassVar[str] = SUDO_FIELD_NAME
executor_host: NonEmptyStr = Field(
validation_alias=EXECUTOR_HOSTS_INPUT_NAME, exclude=True
)
Expand Down
10 changes: 6 additions & 4 deletions app/sep/snippets/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@
from typing import cast
from urllib.parse import urlencode

from app.sep.apps.field_names import (
EXECUTOR_HOST_FIELD_NAME,
EXTRA_ARGS_FIELD_NAME,
SCRIPT_PREVIEW_FIELD_NAME,
SUDO_FIELD_NAME,
)
from app.sep.apps.framework.rules import (
evaluate_conditional_rules,
extract_forbidden_field_gate_plan,
Expand All @@ -54,19 +60,15 @@
ColumnFormat,
DateTimeField,
EXECUTION_HOST_LABEL,
EXECUTOR_HOST_FIELD_NAME,
FloatField,
FormSection,
HostField,
IntegerField,
ListView,
SCRIPT_PREVIEW_FIELD_NAME,
ScriptPreviewField,
StringField,
SUDO_FIELD_NAME,
)
from app.sep.snippets.config import SnippetSudoOption
from app.sep.snippets.models.constants import EXTRA_ARGS_FIELD_NAME
from app.sep.snippets.models.meta import (
SnippetMetaParameter,
SnippetMetaParameterType,
Expand Down
1 change: 1 addition & 0 deletions changelog.d/SEP-1715.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
A snippet or script whose frontmatter declares a parameter named "executor_host", "sudo" or "script_preview" no longer fails to render its execution form with an opaque "duplicate field name(s) across form sections" error. Those three names are now reserved alongside "extra_args": a script declaring any of them fails parameter validation with a message naming the parameter, and cannot be executed until it is renamed unless invalid parameters are configured to be ignored. Reservation is unconditional and applies to every script app, so "sudo" is reserved even on a script configured never to use sudo. Dipper Data Collection now refuses to dispatch a collector script carrying invalid frontmatter parameters, matching the snippets and disk-backed script apps, instead of running it without the arguments its author declared; the refusal lists the validation messages, which that app does not show on the form.
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,7 @@ order-by-type = false
"test_*.py" = ["S", "ARG001", "ARG002", "ANN", "SLF001"]
"conftest.py" = ["S", "ARG001", "ARG002"]
"tests/app/sep/apps/framework/contract_suite.py" = ["S"] # pytest contract-suite mixin uses bare assert
"tests/app/sep/form_schema_utils.py" = ["S"] # shared form-schema assertion helper uses bare assert
"scripts/*.py" = [
"T201", # CLI scripts use print() for user-facing output
"S603", # CLI scripts coordinate git/gh/make subprocesses with literal arg lists
Expand Down
Loading
Loading