Skip to content
Lint

fix(ipv6): CALIPSO make() writes a corrupt length on overflow and raises raw errors for a bad pad #961

Sign in to view logs

fix(ipv6): CALIPSO make() writes a corrupt length on overflow and raises raw errors for a bad pad

fix(ipv6): CALIPSO make() writes a corrupt length on overflow and raises raw errors for a bad pad #961

Workflow file for this run

.github/workflows/lint.yml at c228007
name: "Lint"
# The four linters this repository already maintains -- vermin, pylint, mypy and
# bandit -- had Makefile targets and no CI presence at all, so nothing noticed
# when their output changed. (`isort` does appear in cron-vendor.yml, but as a
# formatter that rewrites the generated constants, not as a check.) This workflow
# is where they run.
#
# Every one of them is advisory -- `continue-on-error: true` on each step -- and
# that is a measurement result rather than a preference. Each line below is
# pinned to the commit it was taken at, rather than left as a bare count: #753
# was filed when three of the four had already drifted from the figures this
# comment once recorded (on 15189abff) -- mypy and pylint by the issue's own
# numbers, and vermin too (105 recorded; 106 measured here both at 73f09aecb,
# where #753 was filed, and at 932cb48d1 below, so it had already moved before
# the issue's own text said it "was not re-checked"). Only bandit held. A bare
# number invites exactly this, with no way to tell staleness from disagreement.
# All four happen to be re-measured together below, each from the same flag
# sets the Makefile uses; re-measuring only one tool later should update only
# that line's commit.
#
# bandit 8 findings (7 medium, 1 low, 0 high) exit 1 [932cb48d1]
# mypy 112 errors in 38 files (506 checked) exit 1 [932cb48d1]
# vermin 106 files flagged; needs 3.11, targets 3.6 in vermin.ini exit 1 [932cb48d1]
# pylint 90 E, 4765 W, 542 C -- stable; R and the total are not, see below exit 30 [1a852698b]
#
# pylint's R and total are deliberately not pinned to a single number. Before
# #767 below, repeated runs on the identical tree gave R of 645-680 and totals
# of 6044-6079 at 932cb48d1 (two runs measured here: 659/6058 and 680/6079; a
# cross-review measured four more: 662/647/647/645, totals 6061/6046/6046/6044),
# entirely from `R0401` cyclic-import, whose count depends on module-processing
# order rather than on the code -- `duplicate-code` (400) and every other R
# check held steady across all of them. That range's own basis is not this
# line's, though: every one of those runs predates #767, so each *R* already
# carries the three `R0022` the old flags spent on `Command line` (not just
# the total -- `R0022` is class R), and each *total* carries one `E0013` and
# one `W0012` on top of that: five stale messages in all, `91 + 4766 + 542 +
# R_old` where `R_old` is itself `R_real + 3`. On the corrected basis
# (`90 + 4765 + 542 + R`) that range is really R 642-677, total 6039-6074 --
# not R 645-680, total 6044-6079 as printed above the line predating #767.
#
# After #767, re-measured on that corrected basis at 1a852698b: two runs here
# gave R 662 and 648 (coincidentally sharing a digit with one of the four
# pre-#767 cross-review runs above, which is a different measurement on a
# different basis, not a repeat of it), totals 6059 and 6045, and a
# cross-review measured five more -- 642/643/646/649/659, totals
# 6039/6040/6043/6046/6056. Combined: R 642-662, total 6039-6059
# [1a852698b]. The floor matches the pre-#767 range's own corrected floor
# exactly (642 both ways) because it is the same tree state either side of
# that diff -- the range only looks lower now because three of its messages
# have been deleted, not because the code changed. Already superseded the
# same day it was measured, too: a cross-review measured `R0801
# duplicate-code` alone moving 400 -> 573 on `merge(f31e5114f, 83b58ebda)` --
# this branch merged onto the `main` it was rebased against before #768
# landed there -- taking R to 823 and the total to 6220 on that merge. Still
# true of `main` itself today, since this PR touches no `pcapkit/` file, but
# named for the tree it was actually measured on rather than asserted of
# `main` directly. This range is a moving target to re-measure, never a
# constant to defend.
#
# The pin above moved from 932cb48d1 to 1a852698b because `pcapkit/` is not
# actually unchanged between them, despite what an earlier revision of this
# comment claimed: four files moved (`pcapkit/const/reg/apptype/apptype.py`,
# `pcapkit/foundation/extraction.py`, `pcapkit/protocols/schema/schema.py`,
# `pcapkit/vendor/reg/apptype/apptype.py`; tree 7270ad50 -> 2b9ac808), #758's
# port-validation logic among them. E, W and C read identically either side of
# that diff -- 90/4765/542 both times -- but 1a852698b is the tree actually
# re-measured for this line, so that is what it is pinned to now, rather than
# repeating a byte-identity claim that does not hold.
#
# Four of `PYLINT_FLAGS`' `--disable=` entries named checks pylint has
# genuinely removed -- `old-division`, `no-absolute-import` and
# `input-builtin` (-> `R0022 useless-option-value`, and pylint's own message
# says so: "was removed from pylint") -- plus `eq-without-hash`, which is not
# gone, only unloaded: `pylint.extensions.eq_without_hash` still ships `W1641`,
# but nothing in `--load-plugins=` ever named it, so `--disable=eq-without-hash`
# was as unrecognised as if the check had been removed (-> `W0012
# unknown-option-value`). Same shape of breakage as `emptystring` below, a
# different cause. All four are dropped either way -- loading the fifth plugin
# to enable `eq-without-hash` properly would find nothing today (`W1641` is 0
# over `pcapkit/`), so there is nothing lost in dropping it instead.
#
# `--load-plugins=` also named `pylint.extensions.emptystring`, which no
# longer exists in the installed pylint (-> `E0013 bad-plugin-value` every
# run). Unlike the four above, though, the check it used to provide did not
# stop running when the plugin did: `compare-to-empty-string` was folded into
# the core `refactoring` checker at pylint 3.0 as
# `use-implicit-booleaness-not-comparison-to-string` (C1804), reachable under
# either name with no plugin load at all. #767's own premise -- that the
# broken plugin reference made the check "inert" and it "has never run" -- is
# wrong: `--list-msgs-enabled` run under the old flags (broken plugin
# reference included) and the new ones side by side gives the same 399-line
# enabled-message list either way, and C1804 fires under the old flags on a
# minimal repro despite the `E0013`. So this half of `PYLINT_FLAGS`' change is
# flag hygiene, not enablement: it drops a dead plugin reference and keeps the
# check it used to gate under that check's current name, and the only thing
# that actually changes is that `E0013` (and the other four's Command-line
# noise) stops firing. With the five gone, the job's own grep-based count (the
# `pylint` step below) and this header have nothing left to disagree about on
# the same run -- the figures above are both the raw and the file-scoped
# count, for E, W and C alike.
#
# None of the four is clean, so none can block today: a job that is red the day
# it lands teaches everyone to scroll past red, which costs more than the checks
# are worth. Advisory steps keep the numbers visible (each writes its count to
# the run summary) without that damage, and promoting one to blocking is a matter
# of deleting its `continue-on-error` line once its count reaches zero.
#
# Promotion order, cheapest first:
# * bandit -- 8 findings, and 7 are B104 "binding to all interfaces" on
# `address: ... = '0.0.0.0'` *default arguments* in MH/MIP option builders,
# which bind no socket. This package already uses `# nosec` (16 findings are
# suppressed that way today), so triaging 8 comments is the whole job.
# * mypy -- 24 of the 112 are `unused-ignore`, i.e. `# type: ignore` comments
# that are no longer needed. Deleting those is risk-free and cuts a fifth.
# * vermin -- blocked on a decision, not on cleanup: see the step below.
# * pylint -- not realistically promotable at this rule set. 4356 of its 4765
# warnings are `unused-argument` (2929), `redefined-builtin` (808) and
# `super-init-not-called` (619), all three of which are inherent to the
# schema DSL: protocol fields are legitimately named `next`/`type`/`id`, and
# the `if TYPE_CHECKING: def __init__(...)` stubs exist precisely so as not
# to call `super().__init__`. Narrowing to `--enable=E,F` does not rescue it
# either -- that is still 87 errors, mostly `unsubscriptable-object` (36) and
# `no-member` (31) false positives off the same metaprogramming.
on:
# Deliberately *not* `push: [main]`, which is the trigger the other validation
# workflows here use. Those gate something: a failing unit-test or CodeQL run
# on main is a commit someone must look at. These steps gate nothing yet, so a
# second advisory run per merge -- on a commit that already got one on its pull
# request -- would buy no information while competing for runners that are
# visibly oversubscribed (pull requests on this repository have been sitting at
# 21 queued checks for ten minutes at a time). The weekly schedule covers
# main's own drift, which is the only thing the push trigger would have added.
pull_request:
branches: [main]
schedule:
# Saturdays, an hour clear of the other weekly jobs: CodeQL runs at 02:00,
# Python Compatibility at 04:00 and Vendor Update at 10:00.
- cron: '0 6 * * 6'
workflow_dispatch:
permissions:
contents: read
# Only pull-request runs may be superseded -- see the note in unit-tests.yml. A
# ref-keyed group drops a *pending* run when a newer one joins it, so scheduled
# and manual runs are keyed on the run id to give each one its own group.
concurrency:
group: lint-${{ github.event_name == 'pull_request' && github.ref || github.run_id }}
cancel-in-progress: ${{ github.event_name == 'pull_request' }}
jobs:
lint:
name: Lint
runs-on: ubuntu-latest
timeout-minutes: 30
steps:
- uses: actions/checkout@v7
# One interpreter, not a matrix. Unit Tests sweeps 3.10-3.14 and Python
# Compatibility sweeps the same plus a schedule-only 3.15 leg, because
# they check that the package *runs* everywhere; these tools read the
# source, and running them several times over would produce several
# copies of the same findings for several times the runner cost.
#
# 3.14 specifically, matching cron-vendor.yml, because it is the newest
# non-experimental version in the test matrix and the version the recorded
# counts above were measured on -- so a count that moves means the code
# moved, not the interpreter.
- uses: actions/setup-python@v7
with:
python-version: '3.14'
# `.[all,dev]` rather than a bare install: pylint and mypy resolve imports,
# so findings depend on what is importable. #910 narrowed `all` to core
# addons only (`cli`, `crypto`, `NGAP`), so the `dev` extra it added is
# what puts `dpkt`/`scapy`/`pyshark`/`pypcapfile` back for this job --
# without it, narrowing `all` would have grown the count below rather
# than left it alone. Note `pypcap` and `pypcapfile` are marked
# `python_version < '3.12'` in pyproject.toml and so are absent here by
# design -- that is the source of pylint's 8 `import-error` messages, and
# they will persist until those engines support a current Python.
- name: Install package and lint tools
run: |
set -x
python -m pip install -U pip setuptools wheel
python -m pip install -e .[all,dev]
python -m pip install -U vermin pylint mypy bandit
# Every step below runs the *Makefile* target rather than spelling the
# flags out again, with `RUN=` emptying the `pipenv run` prefix the targets
# use locally. That is what keeps this workflow and a developer's `make
# pylint` running the same check -- restating the flag sets here would let
# the two drift the first time either changed, which is how "passes on my
# machine" starts.
#
# `continue-on-error: true` reports the step's real verdict as a non-
# blocking annotation and lets the following steps run, so one tool's
# findings never hide another's. The `status=$?` dance is needed because
# the default shell is `bash -e`: without it the summary lines after a
# non-zero tool would never execute.
- name: bandit
continue-on-error: true
run: |
set +e
make bandit RUN= 2>&1 | tee bandit.log
status=${PIPESTATUS[0]}
set -e
{
echo '### bandit'
echo '```'
sed -n '/^Run metrics:/,$p' bandit.log || echo 'no run metrics in output'
echo '```'
} >> "$GITHUB_STEP_SUMMARY"
exit $status
- name: mypy
continue-on-error: true
run: |
set +e
make mypy RUN= 2>&1 | tee mypy.log
status=${PIPESTATUS[0]}
set -e
{
echo '### mypy'
echo '```'
tail -n 1 mypy.log
echo '```'
} >> "$GITHUB_STEP_SUMMARY"
exit $status
# `vermin-ci`, not `vermin`: the `vermin` target redirects into temp/ and
# hands the file to an editor, so its exit status is the editor's and a run
# that found violations still succeeds. Same flags, via $(VERMIN_FLAGS).
#
# This one needs a decision rather than a cleanup before it can block, and
# the decision is not CI's to make. vermin.ini sets `targets = 3.6` while
# the code's real floor is 3.11 -- `enum.StrEnum` and
# `enum.show_flag_values` are 3.11 members, and 86 modules import
# `typing_extensions`. pyproject.toml's `requires-python` still claims
# `>=3.6, <4`, so the declared floor and the actual one are five releases
# apart. Whichever way that is resolved -- raise `requires-python`, or
# raise `targets` to match it -- belongs in its own change.
- name: vermin
continue-on-error: true
run: |
set +e
make vermin-ci RUN= 2>&1 | tee vermin.log
status=${PIPESTATUS[0]}
set -e
{
echo '### vermin'
echo '```'
grep -E '^(Minimum required versions|Incompatible versions|Target versions not met):' vermin.log \
|| echo 'no verdict in output'
echo '```'
} >> "$GITHUB_STEP_SUMMARY"
exit $status
# Last because it is by far the slowest -- minutes over ~90k lines -- so
# the three quick verdicts reach the summary before it starts.
- name: pylint
continue-on-error: true
run: |
set +e
make pylint RUN= 2>&1 | tee pylint.log
status=${PIPESTATUS[0]}
set -e
{
echo '### pylint'
echo '```'
printf 'messages by class (C/E/R/W):\n'
grep -oE ': [A-Z][0-9]{4}:' pylint.log | grep -oE '[A-Z]' | sort | uniq -c || echo 'none'
echo '```'
} >> "$GITHUB_STEP_SUMMARY"
exit $status