It was the only uses: in the repo on a branch rather than a tag, so it
took whatever coverallsapp pushed to master, in a job holding
GITHUB_TOKEN with actions: write. A tag is what the rest of the repo
does and is what lets dependabot move it. v2.3.8 still accepts the
inputs used here; path-to-lcov is deprecated there but present.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The job audits all of .github/ but was only triggered by workflows,
actions and the two lint configs, so a change to dependabot.yml -- which
zizmor now checks -- would not run it, and the failure would land on the
next contributor to touch a workflow instead.
Without a token zizmor runs offline, which skips exactly the five audits
that a tag pin depends on: impostor-commit, ref-confusion,
known-vulnerable-actions, stale-action-refs and ref-version-mismatch.
Since this repo pins tags rather than hashes, those are the compensating
control; hand it github.token, which contents: read already covers. It
reports the same nothing today as it did offline.
Run zizmor even when actionlint failed, so both kinds of problem show up
in one run, and correct the note on the low-severity backlog: it is 46
self-repository, 31 artipacked and 1 adhoc-packages, not mostly
artipacked.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Actions are pinned to a tag rather than a commit hash, so a compromised
release reaches whoever updates first; a cooldown gives a bad release
time to be pulled before dependabot offers it.
This is also the one medium zizmor finding in .github/, so it has to be
answered for the lint job to scan the whole directory.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Scans .github/ for template injection, over-broad permissions, and
unpinned actions/images on the same triggers as actionlint.
The whole directory, not just the workflows: the composite actions under
.github/actions/ carry run: blocks of their own and already trigger this
job, but nothing in CI audits them -- actionlint does not shellcheck
composite-action scripts. A template injection planted in save-ccache is
not reported when only .github/workflows/ is scanned, and is a high
finding when .github/ is.
Annotate the ardupilot-dev-ros:latest image in colcon.yml and
test_dds.yml: those jobs continuously test against the dev image
itself, so they need to track "latest" rather than a pinned snapshot
of it, and zizmor's unpinned-image check would otherwise flag that as
unintentional.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The underlying problem is not really new, it was just never noticed by
the analyzer. Previously, `find_grid_cache` would dereference a null
pointer by working with `cache[0]` if it was called when `cache` is
`nullptr`. Now it will just dereference a null pointer directly.
Unfortunately, this is impossible to fix as the function must return a
cache block. Fortunately, nobody calls it in this circumstance.
It is difficult to find enough contiguous free heap space for a large
terrain cache when it is allocated as one large array. This is
especially problematic on STM32 with the variety of heaps across
different RAM regions.
The cache is only accessed by iterating through its blocks in order.
Therefore, using a singly-linked list of blocks is a natural fit as
there is no random access to penalize. The blocks are then allocated
independently so they can be allocated in different areas or regions as
heap availability dictates. There is an order-1% space overhead for the
`next` pointer and extra heap block headers.
This also fixes memory corruption if the user gives a zero or negative
cache size parameter then enables terrain.
This simplifies the check other parts of the system need to make.
This also avoids potential race conditions causing duplicate allocations
and leaks when multiple threads (e.g. scripting) call `height_amsl`.
Requires a slight hack to also allocate when unit tests force-enable the
library. This way it becomes active so the tests work.
Introducing a const reference makes insignificant assembly-level
changes. Use the original form to prevent this as it is obvious no
mutation is happening here. A future PR will revert this change.
Introducing a const reference makes insignificant assembly-level
changes. Use the original form to prevent this as it is obvious no
mutation is happening here. A future PR will revert this change.
Change the mutations to use `Pmut` and change the define to keep the
non-mutations from accidentally mutating.
The mutations take a variety of forms but all appear to preserve
symmetry.
Change the mutations to use `Pmut` and change the define to keep the
non-mutations from accidentally mutating.
All these mutations only touched the diagonals, so no risk of them
having broken symmetry.
Change the mutations to use `Pmut` and change the define to keep the
non-mutations from accidentally mutating.
All these mutations only touched the diagonals, so no risk of them
having broken symmetry.
Change the mutations to use `Pmut` and change the define to keep the
non-mutations from accidentally mutating.
Most of these mutations only touched the diagonals, so no risk of them
having broken symmetry. The zeroing of the quaternion-to-non-quaternion
covariances also does both axes and keeps symmetry.
Change the mutations to use `Pmut` and change the define to keep the
non-mutations from accidentally mutating.
All these mutations only touched the diagonals, so no risk of them
having broken symmetry.
Change their define so they don't use the mutable original, only the
name through a const-cast pointer. There are no writes to the covariance
matrix in these files, so nothing else needs changing.
In preparation for changing the regular `P` name to const, in
preparation for auditing the code so that writes to the matrix keep its
necessary numeric properties.
Sadly there is not a better way than a per-file `#define` to make the
switch. Making `P` a reference to `Pmut` substantially changes the
compiler output. Defining `P` in the header file conflicts with other
includes. Doing the rename at the top of each file allows each file to
be fixed independently.
It is the only HAL that sets `-Wframe-larger-than` so it is the only one
where the limit needs modification to fit the EKF.
ChibiOS additionally does not use Clang, so that check can be dropped.
Unseeded, MAVProxy draws the lost packets from fresh entropy on every run,
so a failure could not be reproduced. A fixed seed loses the same packets
each time; it still leaves gaps to fill.
The listing tests only ever listed a directory they had just made, so the
paths they used were all of one shape. The bug where listing the root
dropped every file lived through all of them because no test named the
root, which is the first thing a file browser asks for.
Cover the shapes a client actually sends: a directory with nothing in it,
and "." for the directory the vehicle was started in.
The long-name test also only listed without times, where the boundary of
what fits a packet sits eleven bytes further out. Run it both ways, with
names either side of the boundary a listing with times has, so that
dropping an entry which no longer fits still cannot end the listing.
These three were skipped pending MAVProxy fixes: continuing a directory
listing from the listing's own state rather than by mutating the last
operation sent, and taking a listing entry's size from the end of the
entry. MAVProxy master now does both, so rather than skipping them
outright they ask MAVProxy whether it can do this and return early if it
cannot.
The test set 50% receive loss and then waited only for "Total size",
which a listing that gave up part-way through would still print. Wait for
the total the files actually add up to instead.
Give every file a different size while doing so: with all of them the same
the total is just a count, so a listing which lost one page and repeated
another still reached it.
MAVFTPListDirectoryWithTimeMAVProxy checks times are asked for by
default and rendered in local time, and that "ftp set list_time 0"
turns them off again. MAVFTPListDirectoryUnknownTimeMAVProxy checks a
file whose time the autopilot does not know shows as "-" rather than as
a date; its directory holds one file, because a listing is printed in
readdir order and an expect for one entry cannot sit behind another.
MAVFTPListDirectoryFallbackMAVProxy makes the timestamped replies
disappear with the module's own pkt_loss_rx and requires the plain
listing to complete once the loss is lifted - the fallback is the whole
point of the new opcode and nothing else exercises it.
MAVFTPListDirectoryLossyRetry drops half the replies and requires the
listing to finish, as it has no retransmit of its own.
MAVFTPListDirectoryWithTimeMAVProxyTabInName is the timestamped
counterpart of the existing tab test, which takes the size and the time
from the end of the entry rather than the size from the front.
That existing test now asks for a plain listing explicitly: against a
MAVProxy which knows the opcode it would otherwise get a time it is not
expecting.
All are skipped, as they need a MAVProxy which is not released yet.
The spec says a virtual directory is named with an @ prefix, that the
recipient maps it to the underlying filesystem, and that a path which is
not there is NAKed FileNotFound. Check each of those: the alias lists what
the log directory holds, a path below it resolves both with and without
the trailing slash the spec's own example carries, an unknown path below
it is NAKed FileNotFound, and a file read through the alias gives back
what was written.
@MAV_LOG has no class of its own to find in the symbol table, so it is
detected by its prefix string, which only the backend table row puts in the
binary. That row is also only built where logs go to a filesystem, so the
option depends on Logging.
MAVFTP defines @MAV_LOG as the flight-stack-independent location for log
files, so that a GCS can find them without being told where a particular
board keeps them. QGroundControl asks for it before anything else, and
falls back to guessing per-firmware paths when it is not there.
It is an alias rather than a filesystem of its own: the backend table
gains a root, and where a row carries one the resolver rewrites a path
under that prefix to sit under that directory before handing it to the
filesystem which serves it. @MAV_LOG points at the local filesystem, under
whatever directory this board logs to, following a custom log directory
the same way AP_Logger_File does.
The rewritten path has to hold the root as well as the path, so the buffer
is sized for the longest path an FTP listing stats - the longest path a
request can carry, a separator and a 255 byte name - underneath the longest
log directory a board has, and a static_assert holds boards to that. A
path too long to rewrite is refused with ENAMETOOLONG before any filesystem
sees it, rather than being truncated into the name of some other file.
That buffer is not on the stack, where every path-based call would pay for
it. It is allocated the first time an alias is used and kept, and a
semaphore is held around each backend call which uses it. rename, the one
call with two paths alive at once, allocates a second buffer for its new
path and frees it afterwards. Paths which are not aliases take neither the
semaphore nor a buffer.
An alias can't sit on LittleFS for now, and @MAV_LOG is not built where
LittleFS is the local filesystem; forcing it on is a build error. LittleFS
holds its lock from opendir() to closedir(), so the semaphore, held across
each call, would be released before that lock, which ChibiOS mutexes do not
allow, and a thread listing an alias directory would wait for the semaphore
while holding the lock another thread holding the semaphore could be
waiting for.
rename now compares the filesystems which serve its two paths rather than
their table rows, since an alias shares its filesystem with other paths,
and sets EXDEV when they differ.
A prefix now has to be the whole of a path's first component, so that
"@MAV_LOG_backup" is not served as "_backup" under the log directory. This
is not particular to the alias: "@SYSfoo" used to be served by @SYS as
"foo", and is now the local file of that name. Nothing in the tree relied
on the old behaviour.
@MAV_LOG, and the alias support with it, is only built where the logs go
to a filesystem at all.
A name containing a tab must come back from ListDirectoryWithTime as a bare
skip entry, with the names around it still listed, while a plain listing
still sends it as it is.
A tab separates the fields of a listing entry, so a name containing one
cannot be represented. The MAVFTP spec (mavlink-devguide#738) has such an
entry sent as a skip entry, which keeps entry offsets consistent, and
ListDirectoryWithTime follows List Directory in this. Plain listings are
left sending the name as they always have.
A listing with times gives a directory the same three fields as a file,
so the listing helper now picks directory entries apart the same way and
the test checks the subdirectory's size and time. The subdirectory gets a
modification time of its own so that an entry carrying some other entry's
time would be caught.
The listing format gives every entry a size, and ListDirectoryWithTime
gives every entry a time as well, with the type character the only thing
distinguishing a directory from a file. We emitted a bare "D<name>" for a
directory instead, so a client parsing the documented three fields found
only one.
A directory has no meaningful size, so it is reported as zero. Listing
with times now has to stat a directory as well; one which cannot be
stat'ed is still listed, with its time reported as unknown.
Plain ListDirectory is unchanged: it still emits the bare "D<name>" it
always has, so no existing client sees a different directory entry.
MAVSDK's server already sends these fields and its client parses them.
QGroundControl assumed a directory entry was a bare name; a fix for that
is in hand.
Lists a directory of files with known sizes and modification times both
with and without times, paging through the listing by entry count as a
GCS does, and checks the entry format, that directories are still bare
D entries, and that a time the autopilot does not know comes back as
the zero the format defines rather than as the FAT epoch.
The listing helpers learn to ask for times; the directory they build
gets modification times to ask about.
The listing format defines an mtime of zero as "the autopilot does not
know when this file was written", but no filesystem we have ever emits
it. FATFS stamps a file with the FAT epoch, 1980-01-01, when it has no
RTC to ask - and RTC_TYPES defaults to GPS only, so a vehicle which has
not had a fix stamps every file it writes that way. Confirmed on a
ZeroOneX6: every file on the card, across dozens of boots, comes back
as 1980-01-01.
Nothing at or before the FAT epoch is a real modification time, so send
the zero the format defines and let the client say it does not know.
Done here rather than in AP_Filesystem_FATFS::stat(), whose st_mtime
also reaches LOG_ENTRY.time_utc and the scripting stat() binding.
This extends MAVLink FTP to support the new opcode that allows to list
files with modification time in UTC.
This is according to the new spec in:
https://github.com/mavlink/mavlink-devguide/pull/701
(cherry picked from commit 4bc2447cb676f36bf0d712798beacf1de69440f3)
The previous thresholds only checked the sign and a 2-degree minimum.
On the ground the pilot's total lean is limited to 10 degrees, split
across both axes by the test's diagonal stick, so the saved trim is
fully determined; check it against that value.
Removes the comment claiming save_trim is a method on RC_Channels, moves
the auto-trim description from auto_cancel to auto_run which it
describes, and drops the extra indentation level auto_start and auto_run
carried over from their previous nesting.
Whitespace and comments only.
The auto-trim scheduler task moved from RC_Channels_Copter::auto_trim_run
to Copter::AHRSTrimming::auto_run, so extract_features.py was reporting
AP_COPTER_AHRS_AUTO_TRIM_ENABLED as absent from binaries which have it.
Pierre KancirandClaude Opus 5
Graham
Thomas Watson
Peter Barker
Julian Oes
yyzh