Skip to content
Merged
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
33 changes: 33 additions & 0 deletions .github/workflows/unit-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -850,6 +850,26 @@ jobs:
apt_install build-essential libpcap-dev
fi

# #2019: from Ubuntu 25.04, apparmor 4.1 ships an enforcing profile for
# /usr/bin/tshark (/etc/apparmor.d/tshark) whose own file rules read only
# /tmp and /usr/share/wireshark, so on an ubuntu-26.04 image `tshark -r`
# is denied every capture under $GITHUB_WORKSPACE -- "You don't have
# permission to read the file", a TSharkCrashException with retcode 3.
# The profile's `include if exists <local/tshark>` is its site hook, so
# one read rule for the checkout goes there, in the profile's own
# `file r <glob>,` form, and `apparmor_parser -r` recompiles the profile
# with that include and replaces the loaded one. 24.04 has no such
# profile, so there this is a no-op. `pypcap-parity` runs the same body.
- name: Let tshark read the checkout past its AppArmor profile
if: env.ENGINE == 'PyShark'
run: |
set -eu
if [ -f /etc/apparmor.d/tshark ]; then
sudo mkdir -p /etc/apparmor.d/local
echo "file r ${GITHUB_WORKSPACE}/**," | sudo tee -a /etc/apparmor.d/local/tshark >/dev/null
sudo apparmor_parser -r /etc/apparmor.d/tshark
fi

- name: Collect this engine's installed .deb files for the cache
if: >-
env.APT_CACHE_SAVE == 'true' && steps.apt-plan.outputs.key != ''
Expand Down Expand Up @@ -1222,6 +1242,19 @@ jobs:
done
exit 1

# #2019: tshark's AppArmor profile on Ubuntu 25.04 and later; see
# `engine-tests`'s step of the same name, whose body this is.
- name: Let tshark read the checkout past its AppArmor profile
id: apparmor
continue-on-error: ${{ matrix.ktlo == true }}
run: |
set -eu
if [ -f /etc/apparmor.d/tshark ]; then
sudo mkdir -p /etc/apparmor.d/local
echo "file r ${GITHUB_WORKSPACE}/**," | sudo tee -a /etc/apparmor.d/local/tshark >/dev/null
sudo apparmor_parser -r /etc/apparmor.d/tshark
fi

- name: Collect the installed .deb files for the cache
if: >-
env.APT_CACHE_SAVE == 'true' && steps.apt-plan.outputs.key != ''
Expand Down
4 changes: 2 additions & 2 deletions docs/source/contributing/releasing.rst
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ Editing ``__version__`` by hand without also moving :file:`CITATION.cff`'s
(``:494-514``) asserts the two agree, and ``create-release.yml``'s
``unit-tests`` job (``:581-597``) calls ``unit-tests.yml`` with
``gate-only: true``, which runs the **full** suite rather than the tiered
subset an ordinary push runs (its ``gate`` job, ``unit-tests.yml:1738-1813``).
subset an ordinary push runs (its ``gate`` job, ``unit-tests.yml:1771-1846``).
That gate runs whenever a release will -- a moved ``__version__`` is exactly
what makes ``version_check``'s evidence read "not yet published" -- and every
publishing job requires it to have succeeded (``:621,808,974,1210``), so a
Expand All @@ -62,7 +62,7 @@ Either run the script, or move both fields by hand in the same commit.
This is the **only** edit a person makes to get a release started -- the tag,
the Release, and every upload are the workflow's job from here. Other
checks run unconditionally on the same path, though none is a step in *this*
process: the ``changelog`` job (``unit-tests.yml:1581-1607``) fails outright if
process: the ``changelog`` job (``unit-tests.yml:1614-1640``) fails outright if
``CHANGELOG.md`` has drifted from its source entry under
:file:`docs/source/changelog/`, and the release-body step warns, without
failing, if that entry's heading still reads "unreleased"
Expand Down
14 changes: 7 additions & 7 deletions docs/source/contributing/workflows.rst
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ the same commit several times over. It also skips ``changes``,
Checks`_ below), which are gated out by their own ``if:`` rather than by having
already run. ``changelog`` carries no
``if:`` at all and runs on every path regardless -- deliberately, per its own
comment (``unit-tests.yml:1569-1575``): ``create-release.yml`` feeds
comment (``unit-tests.yml:1602-1608``): ``create-release.yml`` feeds
``CHANGELOG.md`` to the GitHub Release body, so the release path is exactly
where a drifted file must not go unchecked. Confirmed on run `36210743295
<https://github.com/JarryShaw/PyPCAPKit/actions/runs/36210743295>`__ (a
Expand Down Expand Up @@ -689,7 +689,7 @@ grepping every workflow file for its name:
- Where
* - ``Required checks passed``
- job ``required-checks``
- ``unit-tests.yml:1904``
- ``unit-tests.yml:1937``
* - ``Compat Python 3.11``
- job ``compatibility``, matrix leg ``3.11``
- ``python-compatibility.yml:60``, ``.github/python-versions.toml:20``
Expand All @@ -711,22 +711,22 @@ which added ``Compat Python 3.15`` and removed ``Compat Python 3.10``. Moving a
between the tiers below means editing the ruleset in the same change.

``Required checks passed`` is defined by exactly one job
(``unit-tests.yml:1904``). ``Compat Python`` is defined by one, the
(``unit-tests.yml:1937``). ``Compat Python`` is defined by one, the
``compatibility`` job in ``python-compatibility.yml`` (``:60``), whose matrix
(``:74``) comes from :file:`.github/python-versions.toml`: the required legs
are its supported tier (``python-versions.toml:20-24``), and its KTLO legs
(``python-versions.toml:16-19``) have names ending in ``(KTLO)``, so no required
context matches one. A comment at
``unit-tests.yml:1816`` mentions the string without defining it.
``unit-tests.yml:1849`` mentions the string without defining it.

``Required checks passed`` is an aggregate, not a single check run. The ruleset
requires it and the ``Compat Python 3.11``-``3.15`` legs, nothing else, as
``unit-tests.yml``'s own comment (``unit-tests.yml:1815-1825``) records. It
``unit-tests.yml``'s own comment (``unit-tests.yml:1848-1858``) records. It
``needs:`` the gating jobs of ``unit-tests.yml`` -- ``test``, ``integration``,
``engine-tests`` and ``pypcap-parity``, plus ``project-tests``, the one run of
the project pins, and ``changes`` for the docs-only path -- and checks each
result explicitly
(``unit-tests.yml:1903-1959``). Rulesets match check names literally, with no
(``unit-tests.yml:1936-1992``). Rulesets match check names literally, with no
wildcard, so requiring the matrix cells one by one would mean editing the
ruleset whenever a matrix changes; the aggregate keeps the required list fixed.
It accepts the gating jobs as ``skipped`` only on a docs-only pull request, where
Expand All @@ -735,7 +735,7 @@ It accepts the gating jobs as ``skipped`` only on a docs-only pull request, wher
``python-compatibility.yml``, and a job cannot ``needs:`` a job in another
workflow file. This job runs on Unit Tests' own ``push``/``pull_request``
triggers; the ``gate-only: true`` reusable calls documented above skip it via
``if: ${{ always() && inputs.gate-only != true }}`` (``unit-tests.yml:1906``),
``if: ${{ always() && inputs.gate-only != true }}`` (``unit-tests.yml:1939``),
so it is never produced -- and never expected -- on those paths.

.. _ktlo-tier:
Expand Down
13 changes: 13 additions & 0 deletions docs/source/pcapkit/foundation/engines/3rdparty.rst
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,19 @@ support, as is used by :class:`pcapkit.foundation.extraction.Extractor`.
|pyshark|_'s own resolver for exactly that reason, so a correctly configured
install off :envvar:`PATH` is not refused.

**Read access under AppArmor.** From Ubuntu 25.04, :program:`tshark` runs
confined by an enforcing AppArmor profile, :file:`/etc/apparmor.d/tshark`,
which reads captures only under :file:`/tmp`, so a capture anywhere else
fails with *You don't have permission to read the file* -- which |pyshark|_
raises as ``TSharkCrashException``. The check above cannot see this. To allow
a directory, add a rule for it to the profile's local include and reload the
profile:

.. code-block:: shell

echo 'file r /path/to/captures/**,' | sudo tee -a /etc/apparmor.d/local/tshark
sudo apparmor_parser -r /etc/apparmor.d/tshark

.. autoclass:: pcapkit.foundation.engines.pyshark.PyShark
:no-members:
:show-inheritance:
Expand Down
157 changes: 157 additions & 0 deletions tests/project/test_tshark_apparmor_2019.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
# -*- coding: utf-8 -*-
"""#2019: every job that installs tshark lets it read the checkout past its AppArmor profile.

From Ubuntu 25.04, apparmor 4.1 confines ``/usr/bin/tshark`` with a profile whose own
file rules read only ``/tmp`` and ``/usr/share/wireshark``, so on an ``ubuntu-26.04``
runner ``tshark -r`` is denied every capture under ``$GITHUB_WORKSPACE``. The step
after each tshark install appends one read rule for the checkout to the profile's
``local/tshark`` include and reloads the profile. These tests run that step's real
``run:`` script with ``/etc/apparmor.d`` swapped for a scratch directory and
``sudo``/``apparmor_parser`` stubbed, so the guard, the rule and the reload are all
exercised rather than read.

"""

from __future__ import annotations

import os
import pathlib
import re
import shutil
import subprocess # nosec: B404
import tempfile
import unittest

from tests._workflows import WORKFLOWS, load, steps

WORKFLOW = 'unit-tests.yml'
NAME = 'Let tshark read the checkout past its AppArmor profile'
#: (job, the tshark install step the AppArmor step must directly follow).
SITES = (
('engine-tests', "Install this engine's system packages"),
('pypcap-parity', 'Install libpcap headers, a C toolchain, and tshark'),
('pypcap-parity-ktlo', 'Install libpcap headers, a C toolchain, and tshark'),
)
#: The directory the step edits, replaced by a scratch one in these tests.
APPARMOR = '/etc/apparmor.d'
#: An AppArmor file rule in ``file ACCESS FILEGLOB,`` form granting read on a tree
#: (apparmor.d(5), FILE RULE), the form the shipped tshark profile itself uses.
RULE = re.compile(r'^file r (/\S+)/\*\*,$')
STUBS = {
'sudo': 'exec env "$@"\n',
'apparmor_parser': 'echo "$@" >> "$STUB_LOG"\n',
}


def apparmor_step(job_id: 'str') -> 'dict[str, object]':
"""Job ``job_id``'s AppArmor step, as the workflow loader parses it."""
for item in steps(WORKFLOW, job_id):
if item.get('name') == NAME:
return item
raise AssertionError(f'{job_id} has no step named {NAME!r}')


class TestEveryTsharkInstallIsFollowed(unittest.TestCase):
"""The step sits right after each tshark install, with one body everywhere."""

def test_the_step_directly_follows_each_install(self) -> None:
"""Each tshark install is followed at once by the step, before any test runs."""
for job_id, install in SITES:
with self.subTest(job=job_id):
names = [item.get('name') for item in steps(WORKFLOW, job_id)]
self.assertIn(install, names)
self.assertEqual(names[names.index(install) + 1], NAME)

def test_every_job_installing_tshark_is_covered(self) -> None:
"""A new tshark install elsewhere in the workflows needs the step too."""
found = set()
for path in sorted(WORKFLOWS.glob('*.yml')):
for job_id, spec in (load(path).get('jobs') or {}).items():
for item in spec.get('steps') or ():
if re.search(r'apt-get install[^\n]*\btshark\b|apt_install tshark\b',
str(item.get('run', ''))):
found.add((path.name, job_id))
self.assertEqual(found, {(WORKFLOW, job_id) for job_id, _ in SITES})

def test_one_body_everywhere(self) -> None:
"""The jobs run one script, so a fix to it cannot reach only some of them."""
bodies = {job_id: apparmor_step(job_id)['run'] for job_id, _ in SITES}
self.assertEqual(len(set(bodies.values())), 1, bodies)

def test_engine_tests_runs_it_only_for_pyshark(self) -> None:
"""Only the PyShark cell installs tshark, so only it reloads the profile."""
self.assertEqual(apparmor_step('engine-tests').get('if'), "env.ENGINE == 'PyShark'")

def test_a_ktlo_parity_leg_is_not_failed_by_it(self) -> None:
"""Every parity leg runs it, and a KTLO leg's failure there stays a warning (#1672)."""
for job_id in ('pypcap-parity', 'pypcap-parity-ktlo'):
with self.subTest(job=job_id):
item = apparmor_step(job_id)
self.assertNotIn('if', item)
self.assertEqual(item.get('continue-on-error'), '${{ matrix.ktlo == true }}')


class TestTheStepRuns(unittest.TestCase):
"""The step's real script, against a scratch ``/etc/apparmor.d``."""

def setUp(self) -> None:
if shutil.which('bash') is None: # pragma: no cover
self.skipTest('bash is not available')
tmp = tempfile.TemporaryDirectory(prefix='pcapkit-2019-') # pylint: disable=consider-using-with
self.addCleanup(tmp.cleanup)
self.tmp = pathlib.Path(tmp.name)
self.bin = self.tmp / 'bin'
self.bin.mkdir()
for command, body in STUBS.items():
path = self.bin / command
path.write_text('#!/bin/bash\n' + body, encoding='utf-8')
path.chmod(0o755)
self.etc = self.tmp / 'apparmor.d'
self.etc.mkdir()
self.log = self.tmp / 'parser.log'
self.workspace = '/home/runner/work/PyPCAPKit/PyPCAPKit'

def run_step(self) -> 'int':
"""Run the step's script against the scratch directory; return its status."""
script = str(apparmor_step('engine-tests')['run'])
self.assertEqual(script.count(APPARMOR), 4,
f'{NAME!r} no longer names {APPARMOR} four times')
script = script.replace(APPARMOR, str(self.etc))
env = dict(os.environ, PATH=f'{self.bin}{os.pathsep}{os.environ["PATH"]}',
STUB_LOG=str(self.log), GITHUB_WORKSPACE=self.workspace)
return subprocess.run(['bash', '-e', '-c', script], env=env, check=False,
capture_output=True, text=True).returncode

def parser_calls(self) -> 'list[str]':
"""The arguments of every stubbed ``apparmor_parser`` call."""
return self.log.read_text(encoding='utf-8').splitlines() if self.log.exists() else []

def test_with_the_profile_the_checkout_is_allowed_and_the_profile_reloaded(self) -> None:
"""26.04: one read rule for the workspace lands in local/tshark, then ``-r`` reloads."""
(self.etc / 'tshark').write_text('include if exists <local/tshark>\n', encoding='utf-8')
self.assertEqual(self.run_step(), 0)
lines = (self.etc / 'local' / 'tshark').read_text(encoding='utf-8').splitlines()
self.assertEqual(len(lines), 1, lines)
match = RULE.match(lines[0])
self.assertIsNotNone(match, lines[0])
self.assertEqual(match.group(1), self.workspace) # type: ignore[union-attr]
self.assertEqual(self.parser_calls(), [f'-r {self.etc}/tshark'])

def test_an_existing_local_file_is_appended_to(self) -> None:
"""A site's own additions in local/tshark are kept."""
(self.etc / 'tshark').write_text('include if exists <local/tshark>\n', encoding='utf-8')
(self.etc / 'local').mkdir()
(self.etc / 'local' / 'tshark').write_text('file r /srv/**,\n', encoding='utf-8')
self.assertEqual(self.run_step(), 0)
lines = (self.etc / 'local' / 'tshark').read_text(encoding='utf-8').splitlines()
self.assertEqual(lines, ['file r /srv/**,', f'file r {self.workspace}/**,'])

def test_without_the_profile_it_is_a_no_op(self) -> None:
"""24.04: no profile, so nothing is written and nothing is reloaded."""
self.assertEqual(self.run_step(), 0)
self.assertFalse((self.etc / 'local').exists())
self.assertEqual(self.parser_calls(), [])


if __name__ == '__main__':
unittest.main()
Loading