Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sanitize input and HTML output better #430

Merged
merged 11 commits into from
Dec 16, 2024
1 change: 1 addition & 0 deletions src/feditest/cli/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ def create_plan_from_session_and_constellations(args: Namespace) -> TestPlan | N
constellations = create_constellations(args)

plan = TestPlan(session, constellations, args.name)
plan.properties_validate()
plan.simplify()
return plan

Expand Down
6 changes: 3 additions & 3 deletions src/feditest/nodedrivers/ubos.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
"""
Nodes managed via UBOS Gears https://ubos.net/
Nodes managed via UBOS Gears https://ubos.net/docs/gears/
"""
from abc import abstractmethod
import hashlib
Expand Down Expand Up @@ -440,10 +440,10 @@ def _provision_node(self, rolename: str, config: NodeConfiguration, account_mana

if config._rshcmd:
if self._exec_shell('which ubos-admin', config._rshcmd).returncode:
raise OSError(f'{ type(self).__name__ } with an rshcmd requires UBOS Gears on the remote system (see ubos.net).')
raise OSError(f'{ type(self).__name__ } with an rshcmd requires UBOS Gears on the remote system (see https://feditest.org/glossary/ubosgears/).')
else:
if not shutil.which('ubos-admin'):
raise OSError(f'{ type(self).__name__ } without an rshcmd requires a local system running UBOS Gears (see ubos.net).')
raise OSError(f'{ type(self).__name__ } without an rshcmd requires a local system running UBOS Gears (see https://feditest.org/glossary/ubosgears/).')

if account_manager is None:
raise RuntimeError(f'No AccountManager set for rolename { rolename } with UbosNodeDriver { self }')
Expand Down
96 changes: 94 additions & 2 deletions src/feditest/testplan.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@
import msgspec

import feditest
from feditest.utils import hostname_validate, FEDITEST_VERSION

from feditest.utils import hostname_validate, symbolic_name_validate, FEDITEST_VERSION

class InvalidAccountSpecificationException(Exception):
"""
Expand Down Expand Up @@ -143,6 +142,43 @@ class TestPlanConstellationNode(msgspec.Struct):
non_existing_accounts: list[dict[str, str | None]] | None = None


def properties_validate(self) -> None:
"""
Validate properties for correctness and safety.
"""
# Unclear whether we can validate the values of the dicts; currently not done.

if self.parameters:
for par_name, _ in self.parameters.items():
if not par_name:
raise ValueError('Invalid TestPlanConstellationNode parameter name: cannot be empty')

if not symbolic_name_validate(par_name):
raise ValueError(f'Invalid TestPlanConstellationNode parameter name: { par_name }')

if self.accounts:
for account in self.accounts:
if 'role' not in account:
raise ValueError('No role name given in account')

if not account['role']:
raise ValueError('Invalid TestPlanConstellationNode account role name: cannot be empty')

if not symbolic_name_validate(account['role']):
raise ValueError(f'Invalid role name in account: "{ account["role" ]}')

if self.non_existing_accounts:
for non_existing_account in self.non_existing_accounts:
if 'role' not in non_existing_account:
raise ValueError('No role name given in non_existing_account')

if not non_existing_account['role']:
raise ValueError('Invalid TestPlanConstellationNode non_existing_account role name: cannot be empty')

if not symbolic_name_validate(non_existing_account['role']):
raise ValueError(f'Invalid role name in non_existing_account: "{ non_existing_account["role" ]}')


@staticmethod
def load(filename: str) -> 'TestPlanConstellationNode':
"""
Expand Down Expand Up @@ -224,6 +260,18 @@ class TestPlanConstellation(msgspec.Struct):
roles : dict[str,TestPlanConstellationNode | None] # can be None if used as template
name: str | None = None

def properties_validate(self) -> None:
"""
Validate properties for correctness and safety.
"""
for role_name, role_value in self.roles.items():
if not symbolic_name_validate(role_name):
raise ValueError(f'Invalid TestPlanConstellation role name: { role_name }')
if role_value:
role_value.properties_validate()

# Not checking self.name: can be any human-readable name


@staticmethod
def load(filename: str) -> 'TestPlanConstellation':
Expand Down Expand Up @@ -288,6 +336,21 @@ class TestPlanTestSpec(msgspec.Struct):
skip: str | None = None # if a string is given, it's a reason message why this test spec should be skipped


def properties_validate(self) -> None:
"""
Validate properties for correctness and safety.
"""
# Not checking self.name: can be any human-readable name

if self.rolemapping:
for role_mapping_key, role_mapping_value in self.rolemapping.items():
if not symbolic_name_validate(role_mapping_key):
raise ValueError(f'Invalid TestPlanTestSpec role mapping key: { role_mapping_key }')

if not symbolic_name_validate(role_mapping_value):
raise ValueError(f'Invalid TestPlanTestSpec role mapping value: { role_mapping_value }')


def __str__(self):
return self.name

Expand Down Expand Up @@ -348,6 +411,16 @@ class TestPlanSessionTemplate(msgspec.Struct):
name: str | None = None


def properties_validate(self) -> None:
"""
Validate properties for correctness and safety.
"""
for test in self.tests:
test.properties_validate()

# Not checking self.name: can be any human-readable name


@staticmethod
def load(filename: str) -> 'TestPlanSessionTemplate':
"""
Expand Down Expand Up @@ -412,6 +485,25 @@ class TestPlan(msgspec.Struct):
feditest_version: str = FEDITEST_VERSION


def properties_validate(self) -> None:
"""
Validate properties for correctness and safety.
"""
if self.session_template is None:
raise ValueError('No session_template in TestPlan')
self.session_template.properties_validate()

for constellation in self.constellations:
constellation.properties_validate()

# Not checking self.name: can be any human-readable name
if self.type != 'feditest-testplan':
raise ValueError(f'TestPlan type is not feditest-testplan: "{ self.type }".')

if not symbolic_name_validate(self.feditest_version):
raise ValueError(f'Invalid TestPlan FediTest version: "{ self.feditest_version }".')


def simplify(self) -> None:
"""
If possible, simplify this test plan.
Expand Down
8 changes: 4 additions & 4 deletions src/feditest/testruntranscriptserializer/html.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ def __init__(self, template_path: str):
self.template_path = [ os.path.join(os.path.dirname(__file__), "templates/testplantranscript_default") ]

self.jinja2_env = jinja2.Environment(
loader=jinja2.FileSystemLoader(self.template_path)
loader=jinja2.FileSystemLoader(self.template_path),
autoescape=jinja2.select_autoescape()
)
self.jinja2_env.filters["regex_sub"] = lambda s, pattern, replacement: re.sub(
pattern, replacement, s
Expand All @@ -88,11 +89,10 @@ def write(self, transcript: TestRunTranscript, dest: str | None):
permit_line_breaks_in_identifier=lambda s: re.sub(
r"(\.|::)", r"<wbr>\1", s
),
local_name_with_tooltip=lambda n: f'<span title="{ n }">{ n.split(".")[-1] }</span>',
local_name_with_tooltip=lambda n: f'<span title="{ html.escape(n) }">{ n.split(".")[-1] }</span>',
format_timestamp=lambda ts: ts.strftime("%Y:%m:%d-%H:%M:%S.%fZ") if ts else "",
format_duration=lambda s: str(s), # makes it easier to change in the future
len=len,
html_escape=lambda s: html.escape(str(s))
len=len
)

try:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@
<html lang="en">
<head>
{% include "partials/shared/head.jinja2" %}
<title>{{ transcript.plan.name }} | Feditest</title>
<title>{{ transcript.plan.name | e }} | Feditest</title>
</head>
<body>
<header class="feditest title">
<h1><span class="prefix"><a href="https://feditest.org/">Feditest</a> Summary Report:</span> {{ transcript.plan.name }}</h1>
<h1><span class="prefix"><a href="https://feditest.org/">Feditest</a> Summary Report:</span> {{ transcript.plan.name | e }}</h1>
<p class="id">{{ transcript.id }}</p>
</header>
{% include "partials/shared/mobile.jinja2" %}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
{% macro column_headings(first) %}
<tr>
<th>{{ first }}</th>
<th>{{ first | e }}</th>
{%- for run_session in transcript.sessions %}
{%- set constellation = run_session.constellation %}
<th>
<div class="title">
<a href="{{ session_file_path(run_session) }}">
<dl class="roles">
{%- for role, node in constellation.nodes.items() %}
<dt>{{ role }}</dt>
<dd>{{ node.node_driver }}</dd>
<dt>{{ role | e }}</dt>
<dd>{{ node.node_driver | e }}</dd>
{%- endfor %}
</dl>
</a>
Expand All @@ -34,9 +34,9 @@
{%- for test_index, ( _, test_meta ) in enumerate(sorted(transcript.test_meta.items())) %}
<tr>
<td class="namedesc">
<span class="name">{{ permit_line_breaks_in_identifier(test_meta.name) }}</span>
<span class="name">{{ permit_line_breaks_in_identifier(test_meta.name) | e }}</span>
{%- if test_meta.description %}
<span class="description">{{ test_meta.description }}</span>
<span class="description">{{ test_meta.description | e }}</span>
{%- endif %}
</td>
{%- for session_index, run_session in enumerate(transcript.sessions) %}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,14 @@
{%- for key in ['username', 'hostname', 'platform'] -%}
{%- if getattr(transcript,key) %}
<tr>
<th>{{ key.capitalize() }}</th>
<td>{{ getattr(transcript, key) }}</td>
<th>{{ key.capitalize() | e }}</th>
<td>{{ getattr(transcript, key) | e }}</td>
</tr>
{%- endif %}
{%- endfor %}
<tr>
<th>Feditest&nbsp;version</th>
<td>{{ getattr(transcript, 'feditest_version') }}</td>
<td>{{ getattr(transcript, 'feditest_version') | e }}</td>
</tr>
</tbody>
</table>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{%- if result %}
<td class="status {{ result.css_classes() }} moreinfo" id="test-{{ session_index }}-{{ test_index }}">
<div>{{ result.short_title() }}</div>
<div>{{ result.short_title() | e }}</div>
</td>
{%- else %}
<td class="status passed" id="test-{{ session_index }}-{{ test_index }}">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
</tr>
{%- for spec_level in [ feditest.SpecLevel.SHOULD, feditest.SpecLevel.IMPLIED, feditest.SpecLevel.UNSPECIFIED ] %}
<tr>
<th>{{ spec_level.formatted_name }}</th>
<th>{{ spec_level.formatted_name | e }}</th>
{%- for interop_level in feditest.InteropLevel %}
<td class="status {{ spec_level.name.lower() }} {{ interop_level.name.lower() }} moreinfo">
<div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
{%- if getattr(transcript,key) %}
<tr>
<th>{{ key.capitalize() }}</th>
<td>{{ getattr(transcript, key) }}</td>
<td>{{ getattr(transcript, key) | e }}</td>
</tr>
{%- endif %}
{%- endfor %}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
<div class="roles">
{%- for role_name, node in run_session.constellation.nodes.items() %}
<div class="role">
<h3 class="name">{{ role_name }}</h3>
<h3 class="name">{{ role_name | e }}</h3>
<div class="driver">{{ local_name_with_tooltip(node.node_driver) }}</div>
<div class="app">{{ node.appdata['app'] }}</div>
<div class="appversion">{{ node.appdata['app_version'] or '?'}}</div>
<div class="app">{{ node.appdata['app'] | e }}</div>
<div class="appversion">{{ node.appdata['app_version'] or '?' | e }}</div>
{%- if node.parameters %}
<table class="parameters">
<thead>
Expand All @@ -16,8 +16,8 @@
<tbody>
{%- for key, value in node.parameters.items() %}
<tr>
<td class="key">{{ key }}</td>
<td class="value">{{ value }}</td>
<td class="key">{{ key | e }}</td>
<td class="value">{{ value | e }}</td>
</tr>
{%- endfor %}
</tbody>
Expand All @@ -33,9 +33,9 @@
{%- set plan_test_spec = transcript.plan.session_template.tests[run_test.plan_test_index] %}
{%- set test_meta = transcript.test_meta[plan_test_spec.name] %}
<div class="test" id="test-{{ test_index }}">
<h4><span class="prefix">Test:</span> {{ test_meta.name }}</h4>
<h4><span class="prefix">Test:</span> {{ test_meta.name | e }}</h4>
{%- if test_meta.description %}
<div class="description">{{ test_meta.description}}</div>
<div class="description">{{ test_meta.description | e }}</div>
{%- endif %}
<p class="when">Started {{ format_timestamp(run_test.started) }}, ended {{ format_timestamp(run_test.ended) }} (duration: {{ format_duration(run_test.ended - run_test.started) }})</p>
{%- with result=run_test.worst_result %}
Expand All @@ -44,9 +44,9 @@
{%- for test_step_index, run_step in enumerate(run_test.run_steps or []) %}
<div class="step">
{% set test_step_meta = test_meta.steps[run_step.plan_step_index] %}
<h5><span class="prefix">Test step:</span> {{ test_step_meta.name }}</h5>
<h5><span class="prefix">Test step:</span> {{ test_step_meta.name | e }}</h5>
{%- if test_step_meta.description %}
<div class="description">{{ test_step_meta.description}}</div>
<div class="description">{{ test_step_meta.description | e }}</div>
{%- endif %}
<p class="when">Started {{ format_timestamp(run_test.started) }}, ended {{ format_timestamp(run_test.ended) }} (duration: {{ format_duration(run_test.ended - run_test.started) }})</p>
{%- with result=run_step.result, idmod='step' %}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
{%- if result %}
<div class="status {{ result.css_classes() }} moreinfo">
<label for="details-{{ result.id() }}{{ idmod }}">{{ result.short_title() }}</label>
<label for="details-{{ result.id() }}{{ idmod }}">{{ result.short_title() | e }}</label>
<input id="details-{{ result.id() }}{{ idmod }}" type="checkbox">
<pre class="stacktrace">{{ html_escape(result) }}</pre>
<pre class="stacktrace">{{ result | e }}</pre>
</div>
{%- else %}
<div class="status passed">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@
<html lang="en">
<head>
{% include "partials/shared/head.jinja2" %}
<title>{{ transcript.plan.name }} | Feditest</title>
<title>{{ transcript.plan.name | e }} | Feditest</title>
</head>
<body>
<header class="feditest title">
<h1><span class="prefix"><a href="https://feditest.org/">Feditest</a> Report:</span> {{ transcript.plan.name }}</h1>
<h1><span class="prefix"><a href="https://feditest.org/">Feditest</a> Report:</span> {{ transcript.plan.name | e }}</h1>
<p class="id">{{ transcript.id }}</p>
</header>
{% include "partials/shared/mobile.jinja2" %}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@
<html lang="en">
<head>
{% include "partials/shared/head.jinja2" %}
<title>{{ run_session.name }} | Feditest</title>
<title>{{ run_session.name | e }} | Feditest</title>
</head>
<body>
<header class="feditest title" style="text-align: center">
<h1><span class="prefix"><a href="https://feditest.org/">Feditest</a> Session Report:</span> {{ run_session }}</h1>
<h1><span class="prefix"><a href="https://feditest.org/">Feditest</a> Session Report:</span> {{ run_session | e }}</h1>
<p class="id">{{ transcript.id }} [<a href="{{ matrix_file_path }}">Summary</a>]</p>
</header>

Expand Down
Loading
Loading