deps(deps): bump golang.org/x/crypto from 0.55.0 to 0.56.0 in the minor-updates group across 1 directory - #311
Merged
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Files
|
tis24dev
added a commit
that referenced
this pull request
Sep 7, 2026
Dependabot #311 bumps golang.org/x/crypto from 0.55.0 to 0.56.0, and v0.56.0 requires `go 1.26.0`, so the update raises the `go` directive to exactly that. All eight workflows resolve their toolchain with `go-version-file: go.mod`, so the directive is what CI installs, verbatim. 1.26.0 is the unpatched base release: govulncheck on that PR reports 20 reachable stdlib vulnerabilities with real call sites, among them GO-2026-6218 in net/url, GO-2026-6090 and GO-2026-5856 in crypto/tls, and GO-2026-5972 in encoding/asn1, fixed in 1.26.5 and 1.26.6. Taking the PR as it stands would trade nothing for those: the two issues 0.56.0 actually fixes, GO-2026-6354 and GO-2026-6355, are DoS in the x/crypto/ssh connection layer, and this module only calls NewPublicKey, ParseRecipient and MarshalAuthorizedKey. Raising the floor here instead, to 1.26.8, the current patch. #311 then rebases onto a patched toolchain and carries the x/crypto bump alone. govulncheck on this tree reports 0 reachable vulnerabilities. Two things had to move with the directive: - goRuntimeMinVersion, the floor enforced at startup, which its own guard test pins to the go.mod directive; - the vet failure Go 1.26 surfaces in TestPrepareDecryptedBackup_SuccessPlain: %q applied to decryptSourceType, which is an int, so the assertion printed the value as a quoted rune. It was the wrong verb before vet started saying so. %v is what the sibling assertions in backup_sources_test.go already use. The install and developer docs still pointed at Go 1.25, which can no longer build the module. No release notes entry: the floor only concerns building from source, and the released binaries carry their own toolchain.
Bumps the minor-updates group with 1 update in the / directory: [golang.org/x/crypto](https://github.com/golang/crypto). Updates `golang.org/x/crypto` from 0.55.0 to 0.56.0 - [Commits](golang/crypto@v0.55.0...v0.56.0) --- updated-dependencies: - dependency-name: golang.org/x/crypto dependency-version: 0.56.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: minor-updates ... Signed-off-by: dependabot[bot] <support@github.com>
dependabot
Bot
changed the title
dependabot
Bot
force-pushed
the
dependabot/go_modules/dev/minor-updates-6e77fb2019
branch
from
September 7, 2026 10:02
53a164d to
d58a721
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
dependabot
Bot
deleted the
dependabot/go_modules/dev/minor-updates-6e77fb2019
branch
September 7, 2026 10:08
tis24dev
added a commit
that referenced
this pull request
Sep 8, 2026
* fix: mark the release notes seen however Screen 0 is closed
Screen 0 wrote the seen-flag only on an explicit continue. Esc, q and Ctrl+C left
it unwritten, so an operator who read the notes and closed the screen the way most
people do kept the warning armed. Every scheduled backup afterwards logged
WARNING ProxSave X has unseen release notes.
which ParseLogCounts counts and applyIssueExitCode promotes to exit 1 on an
otherwise clean run, which the daemon reports to Healthchecks as down. That is
issue #305 seen from the manual-upgrade side: the screen did open, and closing it
the natural way did not disarm anything.
Requiring a specific keystroke to disarm an exit code is not a gate, it is a trap.
Nothing is lost by dropping it, because presence is established BEFORE this point
and not by which key was pressed: maybeShowWhatsnew and showWhatsnewScreen both
gate on a real terminal, the post-upgrade hand-off gates on
whatsnewAfterUpgradeInteractive, and --dry-run returns before rendering. An
unattended run never reaches the write.
One exit still does not count, and the distinction is real rather than cosmetic: a
torn-down PARENT context means the screen never ran. That is an external SIGINT or
SIGTERM, which setupRunContextWithSignals maps to ctx cancellation. A Ctrl+C typed
INTO the screen does not land there, because the terminal is in raw mode and
bubbletea reads it as a key that the router turns into tea.Interrupt, so the parent
stays alive and the notes count as seen.
Two tests pinned the old rule and now pin the new one, including the cases the old
contract explicitly excluded (Esc, Ctrl+C, timeout) and the parent-cancelled case
that still must not write. Mutation-verified: restoring the continue-only write
fails seven subtests across both.
This does NOT close #305. The warning still promotes a clean backup to exit 1, and
the paths that never open the screen at all (--upgrade y, the dashboard's deferred
presentation, any non-TTY upgrade) still leave the flag unwritten. What it removes
is the case where the operator did everything right.
* fix: do not count a screen that only timed out as seen
Narrows the previous commit. It wrote the seen-flag on every resolution, timeout
included, on the argument that an unattended run never reaches the write. That
argument does not hold for the timeout.
isTerminalInteractive proves a TERMINAL, not a person: it checks that stdin and
stdout are TTYs and that TERM is neither empty nor dumb. A detached tmux or screen
window, an expect script and an `ssh -t` from a wrapper all satisfy that with
nobody in front of them. There the screen opens, sits untouched, and the
10-minute timeout fires.
Every other resolution is a KEYSTROKE, which is the evidence the gate cannot
supply: continue, Esc, q and Ctrl+C all mean a person was there and closed the
screen. Those still disarm the warning, which is the whole point of the change.
Sitting untouched for ten minutes is the opposite of evidence.
So the rule is: write unless the parent context was torn down (external SIGINT or
SIGTERM) or the screen timed out.
Mutation-verified both ways: letting the timeout write fails the two subtests
that pin it, and going back to writing only on continue fails the Esc and Ctrl+C
cases.
* deps(deps): bump filippo.io/age from 1.3.1 to 1.3.2 in the security-patches group (#310)
deps(deps): bump filippo.io/age in the security-patches group
Bumps the security-patches group with 1 update: [filippo.io/age](https://github.com/FiloSottile/age).
Updates `filippo.io/age` from 1.3.1 to 1.3.2
- [Release notes](https://github.com/FiloSottile/age/releases)
- [Commits](https://github.com/FiloSottile/age/compare/v1.3.1...v1.3.2)
---
updated-dependencies:
- dependency-name: filippo.io/age
dependency-version: 1.3.2
dependency-type: direct:production
update-type: version-update:semver-patch
dependency-group: security-patches
...
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* feat: audit backup.env for duplicated, absent and unknown variables
parseEnvFile resolves a repeated variable with `raw[upperKey] = value`, so the
LAST assignment wins and every earlier one is discarded in silence. Measured on
ten spellings of the same line, that is the only shape that loses an operator
value: spaces around `=`, an `export` prefix, quotes, an inline comment, CRLF, a
lower-case key and a tab indent all parse correctly. A second line wins.
That is enough to explain issue #306. The template ships PERSONAL_SCRIPT_PRE_RUN
and PERSONAL_SCRIPT_POST_RUN as empty lines (templates/backup.env:456-457), so an
operator who adds their own line ABOVE one of them loses it, while the same file
with the two lines swapped works. Two of the reporter's four machines report the
variable as NOT CONFIGURED with an empty path on both the running-daemon and the
current-configuration side, reading the same file the two working machines read.
The block reports three facts, and their levels are the design:
- duplicated: a value the operator wrote is discarded, so WARNING;
- absent: the binary carries the variable in its EMBEDDED template but the file
does not, which can only mean the configuration merge never ran, so WARNING
too. An operator who has not upgraded runs an older binary with an older
template and sees nothing here;
- unknown: a variable the binary does not read, e.g. a misspelled one. Nothing
is discarded by the loader, it was never picked up, so INFO, and it does not
turn the verdict red.
Values are never printed, not even at DEBUG: a duplicated TELEGRAM_BOT_TOKEN
would otherwise put a secret in the log. Line numbers locate it just as well.
parseEnvFile is NOT touched. Its upstream blast radius is CRITICAL (27 symbols,
5 execution flows: install, upgrade, upgrade-finalize, install TUI, decrypt), so
the audit re-reads the file with the same primitives instead - utils.IsComment,
utils.SplitKeyValue, the same `export` rule and the same multiValueKeys and
blockValueKeys sets, including the multi-line block form. The two readers agree by
construction rather than by coincidence, and TestAuditAndLoaderAgreeOnTheWinningLine
pins that across six spellings: whatever the audit calls the winning line must hold
the value the loader kept.
The four variables that CONCATENATE instead of overwriting are excluded, or a
perfectly normal file with two BACKUP_EXCLUDE_PATTERNS lines would be reported.
Position: first statement of validateRunConfig, before printDryRunBootstrapStatus,
which closes the configuration section with a blank line. A discarded value has to
be reported before the effective-settings recap that shows the surviving one as if
nothing were lost. TestTheIntegrityBlockIsWiredAheadOfTheConfigurationSectionEnd
pins the position, which no renderer test can see.
Mutation-verified: removing the multi-value guard, declaring the first assignment
the winner, downgrading absent to INFO, promoting unknown to WARNING, counting
unknown as an issue, dropping the subsystem prefix from the DEBUG lines, moving
the raw counters after the verdict and moving the whole block after the blank
line are each caught by a test.
Refs #306
* style: say "mode" for the email delivery setting, not "method"
The notification-init block used two words for the same kind of thing:
✓ Email initialized (method: relay)
✓ Healthchecks initialized (mode: centralized)
✓ Telegram initialized (mode: centralized)
Three lines apart, two of them "mode" and one "method". Telegram already shows
the log word is not tied to the variable name: it prints "mode" for a setting the
configuration calls BOT_TELEGRAM_TYPE.
So the whole email-delivery family moves to "mode": the init line, the five
delivery lines in the notifier, the invalid-value error, the two security
dependency notes, the two CLI wizard prompts, the TUI label and the five
documentation files that carry the same phrase.
EMAIL_DELIVERY_METHOD is NOT touched, nor is any other variable: renaming one
would invalidate every backup.env in the field for a wording change. The prose
says "mode" and the variable stays METHOD, which is exactly the arrangement
Telegram has had all along.
Untouched because they are a different concept, not the same word used loosely:
the HTTP method in the webhook notifier, RCLONE_VERIFY_METHOD in cloud
verification, the network reload methods (ifreload vs ifup), the Go syntax error
text in the cron parser and the PVE API's own "methods" JSON field.
The two install characterization transcripts are regenerated, since they pin the
wizard's exact output.
Strings only: the Go diff contains no line that is not a string literal.
* feat: say why a personal script reads as NOT CONFIGURED
NOT CONFIGURED told the operator the verdict and nothing else, and the verdict alone
cannot tell three different files apart: a variable that is not in backup.env, one
assigned with an empty value, and one whose value a later line overwrites. Issue #306
is the third case seen from the outside. Two of the reporter's four machines showed
Personal pre-run script:
Running daemon: NOT CONFIGURED
Current configuration: NOT CONFIGURED
Synchronization: IN SYNC
while he was looking at a file that carries the path, on the same binary and the same
configuration path as the two machines that work. There was nothing in the output to
close that gap, and IN SYNC read as reassurance while both sides were wrong together.
The current-configuration side now names what the file says:
Configuration: NOT CONFIGURED (PERSONAL_SCRIPT_PRE_RUN is not in the file)
Configuration: NOT CONFIGURED (PERSONAL_SCRIPT_PRE_RUN is on line 456 & is empty)
Configuration: NOT CONFIGURED (PERSONAL_SCRIPT_PRE_RUN is on lines 120 and 456 & line 456 wins and is empty)
The evidence comes from the audit added for the integrity block, so the two features
read the file with one scanner and cannot disagree about which line wins. Values are
still never recorded, only whether the winning one is empty.
Only the CURRENT side is annotated. The running daemon read its own copy of the file
when it started, so describing today's file as if it explained the daemon's verdict
would be a guess dressed as evidence.
The note is a new field, NOT the existing Reason: comparePersonalScript compares Reason
between the two sides, so a note only one side can ever carry would turn every IN SYNC
into PATH STATE CHANGED, a warning about a daemon that is fine.
Labels, both renderers:
Running daemon: -> Daemon now:
Current configuration: -> Configuration:
"Current configuration: NOT CONFIGURED" has no subject and, read on its own line, says
that the configuration is not configured. That is how the reporter read it, and he said
so. The block's subject is the script named in the title; the two lines under it say
where, not what.
Mutation-verified: putting the note in Reason, annotating the running side, collapsing
absent and empty into one message, taking the first assignment as the winner, dropping
the state guard, and dropping the note from either renderer are each caught by a test.
Refs #306
* style: shorten the foreign-ancestor trust warning and state it once per owner
The advisory ran past 240 characters and repeated itself. A script under
/home/<user>/<dir> has two foreign-owned ancestors with the SAME uid, and the walk
emitted the same clause for each, so the reporter of issue #306 read:
/home/howard is owned by uid 1000; that owner can replace descendants executed as
daemon uid 0; fs.protected_hardlinks is 1, so that owner cannot hard-link a
root-owned executable into place
and said: "I didn't understand it or what to do about it".
Now:
/home/howard, /home/howard/dd: UID 1000-owned; owner can replace descendants run as
UID 0; fs.protected_hardlinks=1 blocks hard-linking root-owned executables
One clause per OWNER, not per directory: the second directory adds a path to the list,
not a second copy of a trust decision that has not changed. Two different owners are
two different decisions and stay separate.
The paths read shallowest-first, the order the path itself is written in. The walk
collects them deepest-first, so printing them as collected spelled the path backwards.
The three hardlink forms shorten the same way, and the one that has an action keeps it:
fs.protected_hardlinks=1 blocks hard-linking root-owned executables
fs.protected_hardlinks=0 allows hard-linking root-owned executables; set it to 1
fs.protected_hardlinks unreadable: <error>
Wording only. The verdict, the severity and the trust policy are unchanged: a
foreign-owned non-loosely-writable ancestor is still READY WITH WARNING and still
enabled, as documented in docs/SECURITY.md.
Mutation-verified: printing the paths deepest-first, going back to one clause per
directory, dropping the hardlink advisory, dropping the "set it to 1" action,
hardcoding the sysctl value and hardcoding the daemon uid to 0 are each caught by a
test. The last two needed new cases, since every existing one used value=1 and uid=0.
Refs #306
* docs: document the configuration integrity check and the new daemon-status wording
CONFIGURATION.md gains a "Configuration integrity check" section, placed with the file
itself rather than with any one setting, because the rule it documents governs every
variable in the file: a repeated variable is resolved LAST-WINS, and until now nothing
said so anywhere. Thirty-six of the template's 182 variables ship as an empty line, so
adding your own line above one of them silently discards it. That is the shape behind
issue #306, and the section names it with the file laid out as the operator sees it.
It also documents why the three findings carry different levels, that the four
concatenating variables are exempt, that "absent" can only mean the configuration merge
never ran because the template is compiled into the binary, and that values are never
printed at any level.
Two cross-links, both to facts that were already documented separately and now have one
place to meet:
- the legacy key names table, whose seven entries are absent from the embedded template
and therefore listed as unknown variables on every run, which is a cheaper way to find
them than grepping for the left column (verified: all seven);
- TROUBLESHOOTING's "a personal script seems not to run", where a value discarded by a
later assignment now stands alongside the causes that were already listed.
DAEMON.md and TROUBLESHOOTING.md follow the renamed status lines, Running daemon ->
Daemon now and Current configuration -> Configuration, and document the reason the
Configuration line now gives for NOT CONFIGURED, including why the Daemon now line
never carries it.
Docs only. Every anchor in docs/*.md resolves, checked mechanically.
* docs: add the 0.37.0 release notes entry
Screen 0 needs its entry before the first beta of the line, not in the final's release
PR: a beta that ships without one renders the empty state and still writes last_seen,
and that flag finalizes to the same key as the final, so everyone who came through a
beta misses the notes for good.
Three lines, the user-facing half of this line's work:
- the backup.env audit that runs on every run;
- the value silently discarded by a later assignment of the same variable, which is
the reporter's case in issue #306 and the one that actually costs him a backup step;
- daemon status saying why a personal script reads as not configured.
Left out on purpose, per the rules in this file: the renamed status labels, the shorter
trust warning and the email delivery wording. All three are UX polish on something that
already existed, and none changes what the operator can do.
Two actions, both real steps and both conditional on what the run actually reports. The
second points at the dashboard rather than --upgrade-config, and its path is verified
against the live menu: ActionCheckUpgrade renders as "Upgrade" under the "Maintenance"
separator (internal/ui/flows/menu/menu.go:76-79), and its chooser offers "Check config"
(cmd/proxsave/dashboard_upgrade.go:78).
Every line is ASCII and under the 120-character limit; the longest is 117.
* build: raise the Go floor to 1.26.8 so CI installs a patched toolchain
Dependabot #311 bumps golang.org/x/crypto from 0.55.0 to 0.56.0, and v0.56.0
requires `go 1.26.0`, so the update raises the `go` directive to exactly that.
All eight workflows resolve their toolchain with `go-version-file: go.mod`, so
the directive is what CI installs, verbatim. 1.26.0 is the unpatched base
release: govulncheck on that PR reports 20 reachable stdlib vulnerabilities
with real call sites, among them GO-2026-6218 in net/url, GO-2026-6090 and
GO-2026-5856 in crypto/tls, and GO-2026-5972 in encoding/asn1, fixed in 1.26.5
and 1.26.6. Taking the PR as it stands would trade nothing for those: the two
issues 0.56.0 actually fixes, GO-2026-6354 and GO-2026-6355, are DoS in the
x/crypto/ssh connection layer, and this module only calls NewPublicKey,
ParseRecipient and MarshalAuthorizedKey.
Raising the floor here instead, to 1.26.8, the current patch. #311 then rebases
onto a patched toolchain and carries the x/crypto bump alone. govulncheck on
this tree reports 0 reachable vulnerabilities.
Two things had to move with the directive:
- goRuntimeMinVersion, the floor enforced at startup, which its own guard test
pins to the go.mod directive;
- the vet failure Go 1.26 surfaces in TestPrepareDecryptedBackup_SuccessPlain:
%q applied to decryptSourceType, which is an int, so the assertion printed
the value as a quoted rune. It was the wrong verb before vet started saying
so. %v is what the sibling assertions in backup_sources_test.go already use.
The install and developer docs still pointed at Go 1.25, which can no longer
build the module.
No release notes entry: the floor only concerns building from source, and the
released binaries carry their own toolchain.
* deps(deps): bump golang.org/x/crypto from 0.55.0 to 0.56.0 in the minor-updates group across 1 directory (#311)
deps(deps): bump golang.org/x/crypto
Bumps the minor-updates group with 1 update in the / directory: [golang.org/x/crypto](https://github.com/golang/crypto).
Updates `golang.org/x/crypto` from 0.55.0 to 0.56.0
- [Commits](https://github.com/golang/crypto/compare/v0.55.0...v0.56.0)
---
updated-dependencies:
- dependency-name: golang.org/x/crypto
dependency-version: 0.56.0
dependency-type: direct:production
update-type: version-update:semver-minor
dependency-group: minor-updates
...
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* fix: report the block assignment that silently discards a backup path
CUSTOM_BACKUP_PATHS and BACKUP_BLACKLIST accept two shapes, and parseEnvFile
treats them in opposite ways. The single-line form concatenates. The block form
does not: its branch ends with a plain `raw[upperKey] = strings.Join(blockLines,
"\n")`, so it replaces everything the file set before it.
skipsUniquenessCheck exempted both variables from the duplicate check outright,
on a doc comment that stated the opposite of the loader. The shipped template
writes both of them in block form with a comment-only body, so an operator who
adds `CUSTOM_BACKUP_PATHS=/srv/important` above line 395 gets the value wiped by
the template's own block on 396, and the audit prints "Configuration file ok".
That is the failure the 0.37.0 registry entry claims to have closed; it was
closed for ordinary variables only.
discardsAValue replaces the blanket exemption and answers the question the audit
actually needs: does this repetition throw anything away, and which assignment
throws it. Three rules, one per shape the loader implements. The form is now
recorded on each assignment as it is scanned; it was already being detected to
skip the block body, and then discarded.
parseEnvFile is untouched, per the reasoning already in AuditConfigFile's own
comment: its blast radius is the whole install/upgrade surface, and making the
block concatenate would silently change what existing installations back up.
Two rendering defects surfaced with the fix, both from deriving what is lost
instead of being told:
- the sentence computed the discarded lines as "every line but the last". With
a single line AFTER the block that is wrong twice over, because the later
line concatenates onto the block instead of winning, so line 396 was printed
as the winner and as discarded in the same sentence. DuplicatedVariable now
carries Discarded, and how many times a variable is SET is counted separately
from how many values are LOST;
- the debug line said "last assignment wins" as a fixed string. It now names
the winning line.
The warning text itself is unchanged: it is the sentence already shipped for
PERSONAL_SCRIPT_PRE_RUN, and it reads correctly here once the inputs are right.
One more line stopped being true. The debug list is what CAN repeat without
discarding, not what this run left alone, and with a block reported two lines
below it "skipped" contradicted the finding.
TestAuditAgreesWithTheLoader is the guard integrity.go's doc comment has named
since the feature landed, and which did not exist. It pins all four shapes
against the real loader, and it fails on the two block shapes before this change.
* fix: name the assignment in effect, not the last line, for a block variable
Two loose ends from the previous commit.
VariableAssignment.WinningLine still said "the last line", which the block form
makes false: a line after a block concatenates onto it instead of winning over
it, so in a file that sets the variable, then opens a block, then adds one more
line, the value in effect comes from the block and the last line only adds to it.
Assignment() is public and answers "how is this variable written", so it cannot
carry an answer that holds for ordinary variables only.
The rule now lives in one place. replacingAssignment returns the last assignment
that REPLACES rather than concatenates, and both callers use it: discardsAValue
for the warning, and the assignments map for Assignment(). Nothing changes for
PERSONAL_SCRIPT_PRE_RUN and POST_RUN, the only production readers, because an
ordinary variable's last assignment is also its last replacing one.
The 0.37.0 action line told the operator "the last one in the file is the one
that wins". That was the rule the audit itself had wrong, and it is wrong for the
same shape. The warning now names the winning line, so the action points at it
instead of restating a rule with an exception.
* fix: bound the personal-script open so it cannot wedge the scheduler
openPersonalScriptForExecution runs on the caller's goroutine before the timeout
context and the stop channel are ever consulted, and safefs.OpenFileUnderRoot
opens the PARENT DIRECTORY through os.OpenRoot, which is a plain blocking open(2)
(os/root_unix.go, wrapped in ignoringEINTR so a signal only makes it retry). The
O_NONBLOCK the caller passes reaches the final component alone, so it does nothing
for a parent that is a FIFO or that lives on a mount which stopped answering.
The consequence is the one superviseChild's own comment spells out, reached by a
path outside its protection: scheduleLoop never returns so no later backup is
scheduled, run() never reaches wg.Wait() so SIGTERM cannot stop the daemon, and
heartbeatLoop keeps reporting the host green from its own goroutine. The unit
carries neither WatchdogSec nor RuntimeMaxSec, so systemd sees a live process and
Restart=always never fires.
It is not only the dead-mount case. personal_scripts_inspection.go keeps a script
under a foreign-owned ancestor ENABLED as ready-with-warning, and its comment
states what that decision rests on: "what stops them is the execution-time gate
refusing anything not owned by root or the daemon". A FIFO does not beat that
gate, it stops it from ever being reached, so the owner of /home/<user> can wedge
the daemon permanently with one mkfifo. Both starters carry the open, and the
detached one runs from a defer on the shutdown path, so it wedges the way out.
probeWithin already carries the exact contract this needs and already serves
cronDeadlineValue with the same budget: the goroutine it gives up on is abandoned
rather than cancelled, which is the only thing available since a blocking open
cannot be cancelled. If the open does eventually return, its *os.File is left in
the buffered channel nobody reads and os.File's runtime finalizer closes the
descriptor, so no drain is needed.
5 seconds, cronProbeTimeout's value and its reasoning: not an I/O budget, the line
between "slow" and "never". A healthy open measured p50 33us and p99 217us over
1000 runs, four orders of magnitude below it. The asymmetry points the same way -
expiring refuses one script for one run and the daemon carries on, while too tight
a bound would silently skip a script on a working but slow mount, and silence is
this file's error contract.
The two tests use a FIFO as the parent directory, which is the cheapest faithful
stand-in for both trigger shapes, and close the stop channel BEFORE the call so
nothing but a bound on the open can make them return. Both hang without this
change.
* fix: stop calling working configuration an ignored variable
"Unknown" was computed against the template's ACTIVE assignments alone, so a
variable the loader really reads was reported as "is not a known variable and is
ignored" whenever the template does not assign it uncommented. Measured on the
shipped template plus a configured Pushover endpoint and SAFE_PROCESSES: the audit
named all three as ignored while the loader had them in effect, the endpoint built
with its URL and SafeProcesses resolved to [ffmpeg].
That is worse than a cosmetic wrong line. The message tells the operator the value
is dead, and acting on it means deleting a working webhook and its auth token. The
template itself is what leads them there: it documents the per-endpoint variables
as commented examples and instructs the operator to write them by hand, and every
endpoint they add produces another "ignored" line.
Three families the loader reads never appear as an active template assignment, and
each needs its own rule:
- variables the template only DOCUMENTS, as `# SAFE_PROCESSES=""`. Commented
assignments now count as known. Only a bare NAME before the '=' qualifies, so
the prose around them ("# Example: SAFE_PROCESSES=..."), and any commented URL
carrying a query string, are not read as declarations;
- the per-endpoint webhook variables. BuildWebhookConfig derives their names at
runtime as WEBHOOK_<NAME>_<FIELD> where NAME is the operator's, so they cannot
be in the template at all. The FIELD list is closed, which is what keeps the
rule from accepting a misspelling;
- the legacy notification aliases, which getBoolWithLegacyAlias and
getBoolWithFallback still read so a file written before the rename keeps
working. No template has ever carried them, commented or not, so they can only
be listed.
The category still means something: the second test pins that a misspelled
PERSONAL_SCRIPTS_PRERUN and a WEBHOOK_MINE_NOSUCHFIELD are both still reported.
It passed before this change and must keep passing, which is what stops the fix
from being "report nothing".
* feat: make the integrity debug block state every decision it made
The block named its findings but not its reasoning, and the three decisions it
kept silent are exactly the three that were wrong before this line of work: why a
variable outside the template's active assignments was accepted, why one was not,
and which of the two write forms won a duplicate. Reading a wrong verdict meant
reading the source.
Four lines, each answering one of those:
- the template's two halves, `assigns N variables and documents M more as
commented examples`, so the known set can be reconstructed from the block;
- per accepted variable, the RULE that accepted it, in the same words the rule
is documented by: a commented example, a per-endpoint webhook field, a legacy
alias;
- per unknown variable, every gate it failed, in the order they were tried.
"absent from the embedded template" was the OLD rule and, alone, is what made
this category report working configuration as ignored, so the line now says
what was actually checked;
- per duplicate, the FORM that won. An ordinary assignment is last-wins, a block
replaces everything before it and lets a later single line concatenate onto
it. Naming the line without naming the form leaves the reader to work out
which of two opposite rules applied.
Everything stays at DEBUG and the operator-facing lines are untouched, so a run at
the default level reads exactly as before.
* docs: say that an absent variable does promote the run to exit 1
CONFIGURATION.md promised "a missing variable falls back to its default in the
meantime, it does not fail the run", and contradicted itself eight lines later
under "What the block does not do", which already said the WARNING lines count
towards the run's exit code. Measured: two absent variables render three WARNING
lines, ParseLogCounts returns warnings=3, and applyIssueExitCode turns a backup
that did everything right into exit 1, which reporter.go then sends as a "/1"
Healthchecks ping. The check goes down over a configuration file, not a backup.
The behaviour stands, as decided. The paragraph now says what the operator will
actually see, and names the state it puts them in until --upgrade-config is run.
The duplicate section was also out of date with the code as of 9a8c704. It said
four variables are exempt because repeating them concatenates; the exemption is
the FORM, not the name. BACKUP_BLACKLIST and CUSTOM_BACKUP_PATHS also take the
block form the template ships them in, a block replaces everything set before it,
and the audit now reports that. The worked example is the shape an operator hits:
their own line written above the template's block.
* feat: give a legacy variable name its own category instead of calling it unknown
Twelve names are read by the loader as a stand-in for a canonical one, and neither
existing category described them. "Unknown" said the binary does not read the
variable, which is false and, for seven of them, backwards: getStringWithFallback
lists the LEGACY name first, so it wins and a canonical line in the same file has
no effect at all. fe0b1f2 silenced five of the twelve, which left the split with no
logic behind it - four still reported as ignored, two invisible - and broke the
workflow CONFIGURATION.md documents, reading the unknown list to find them.
They now have their own category, carrying the canonical name and which of the two
the loader consults first. legacyAliases is derived from the call sites rather than
from the docs: whichever name is listed first in getStringWithFallback /
getBoolWithFallback is the one that wins, and that is what Wins records.
The level follows the same logic the other three already use, so nothing new is
invented:
- both names set is a WARNING. One of the two lines has no effect, which is the
duplicate's harm under another shape, and the line says which one it is because
it is not always the one the operator would guess: the seven path and storage
pairs let the legacy name win, the five notification aliases let the canonical
one win;
- the legacy name alone is an INFO. It works, nothing is discarded, so the
verdict stays green and the line names the canonical variable to move to.
The debug line adds what the operator-facing one leaves out: which name the loader
consults first, and whether the canonical is assigned too, so the WARNING above it
can be reconstructed without reading the source.
CONFIGURATION.md gains the row, the worked example and the correction: the unknown
row no longer claims a legacy name lands there, and the legacy section says the
check names them all rather than that they appear as unknown variables.
* feat: say when a guest config was applied over a PVE lock
The locked apply refuses a RUNNING guest, and stops there. PVE has a second way
of saying "busy": a lock written into the config itself while a backup, a
migration, a clone or a snapshot runs, and a STOPPED guest carries it just the
same. The helper loaded the config inside the lock but discarded the return
value, so $conf->{lock} was never looked at and such a guest was overwritten with
nothing said. PVE's own writers refuse in that state.
Overriding is the decided behaviour, not an oversight to correct: that marker is
frequently left behind by an operation that died, there is no way to tell a stale
one from a live one by reading the config, and refusing would block the operator
in the very case a restore exists for. What was wrong is that the override was
invisible.
The helper now reports the lock it wrote over on its own line, and the apply puts
it in front of the operator next to the line that says the config was applied:
WARNING Applied VM/CT config 101 (webserver) over a "backup" lock - PVE had the
guest marked busy and that marker is now gone
WARNING rather than INFO because it belongs in the run's warning summary rather
than among the guests that applied cleanly; the restore path does not run the
warning-to-exit-1 promotion, so nothing else moves. A guest carrying no lock gains
no line at all, which the second test pins.
The lock name comes out of a guest config file, so it is accepted only as a bare
lowercase word before it reaches a log line.
guestDisplay now holds the one way a guest is named in these lines, so the applied
line and the override line above it cannot drift apart.
* fix: name the storage keys the update schema refused
pveshSetDroppingRefusedKeys returns the keys it had to drop, and its own doc
comment says a caller that stays silent about them loses the fact that their
staged values were NOT applied. The guest arm reports them in three distinct
shapes. The storage arm discarded the list with a blank identifier and had two,
so an operator read "Updated existing storage definition nfs-backup" over a
definition that is not in the staged state, with only a DEBUG line naming what
had been dropped.
The all-refused case was worse. It rendered as "already matches every settable
key", which says the opposite of what happened: nothing reached the node, so the
definition does not match, and the staged values are not in effect. That wording
belongs to the OTHER shape reaching the same branch, a block whose only keys were
the create-only header ones, where it is true. dropped is what tells the two
apart, and the doc comment that called them the same fact is corrected.
The storage arm now has the guest arm's three outcomes, in its words:
WARNING Applied nothing for storage nfs-backup: the update schema refuses every
staged key (server, export, content)
WARNING Updated existing storage definition nfs-backup without server: the
update schema refuses those keys, so their staged values are not applied
INFO Updated existing storage definition nfs-backup
Reachability is recorded in this file already: the convergence comment cites an
nfs block probed live on PVE 9.1.9 with 3 settable keys and 2 refused.
The applied/failed counts are deliberately untouched. The guest arm counts an
all-refused guest as a failure and the storage arm counts it as applied, which is
a second inconsistency between the two, but changing it moves the run summary and
is a separate decision.
* fix: stop a sysctl change reading as a personal-script path state change
comparePersonalScript compares the Reason strings of the two sides, and the
hard-link advisory inside them is a live read of /proc/sys/fs/protected_hardlinks.
Change that setting while the daemon runs and the strings differ, so daemon-status
reports PATH STATE CHANGED SINCE STARTUP, "path ownership or mode changed after
daemon startup", over a path where nothing moved. The same false verdict comes out
of /proc being briefly unreadable, which turns the clause into "unreadable: ...".
The rule was already written down, in the doc comment of the field next to it:
Assignment is deliberately kept out of Reason because "comparePersonalScript
compares Reason between the two sides, so a note that only one side can ever carry
would turn every IN SYNC into PATH STATE CHANGED". The advisory is not a fact about
the path either, and it was left in.
It now rides its own field, and the path facts it used to be glued to are compared
alone - Components already carries owner and mode per directory, so nothing about
the comparison is weakened.
A daemon started before this change wrote the advisory INTO its stored Reason, so
both the conversion that reads that state and the comparison itself normalise it
away. Without that, every such daemon would report the very verdict this fixes
until it restarted.
Two lines instead of one, the second next to the side it describes:
WARNING Configuration: READY WITH WARNING (/home/me/dd/hook.sh): /home/me,
/home/me/dd: UID 1000-owned; owner can replace descendants run as UID 0
WARNING Configuration: fs.protected_hardlinks=1 blocks hard-linking root-owned
executables
The mitigation still travels with the advisory and never alone, which is what the
test named for it pins; only the carrier changed.
The debug line names WHICH of the three compared facts moved, so a reader who
disagrees with "path ownership or mode changed" can see what produced it:
DEBUG personal script synchronization: verdict=path-state-changed
difference=path components differ in owner or mode
* fix: emit the synchronization debug before the verdict it explains
The difference between the two sides is what was DETECTED; the verdict is what was
concluded from it. Printing the conclusion first leaves the reader scrolling
backwards for the evidence, and the repo already holds the opposite rule for the
configuration block, in TestIntegrityDebugCountsPrecedeTheVerdict.
The two other lines added in this line of work were already in that order: the
guest lock override emits its DebugStep before the warning, and every debug line of
the integrity block precedes the operator lines it explains.
* fix: stop calling a reloaded dashboard's own exit code a reload failure
After an upgrade the dashboard closes and relaunches itself, and the parent waits
for that process. It treated ANY non-zero result as a failed reload, so an operator
who ran a backup with a warning inside the reloaded dashboard read
ERROR Dashboard reload failed after upgrade: exit status 1
on exit. The reload had worked; they had just spent a whole session in the process
it started. The 1 was their own backup's.
The same call also threw the code away: the parent returned success whatever the
child did, so the session's outcome never reached the shell.
childReachedItsOwnExit already draws the line this needs, and its doc comment
explains why only the error can: an *exec.ExitError is a child that ran and exited,
anything else means this process never saw a child reach an exit of its own. So a
child exit is now carried out as the exit code, silently, and only a relaunch that
never started keeps the error line and reports a failure.
The debug workflow closes as successful in the first case too. The reload succeeded;
marking it failed because the session that followed ended with a warning was the
same conflation one level down.
relaunchInstalledDashboard is untouched and still returns an error for a non-zero
child, which is what its own test pins: the low-level call reports what happened,
and deciding what it MEANS is the caller's job.
* feat: name the VMID an aborted registration may have left reserved
Registering a guest that does not exist yet claims the VMID cluster-wide with a
create-locked placeholder FIRST, then writes the staged conf. The helper cleans the
placeholder up only when that second write returns an error. A cancelled restore
kills the helper outright, so nothing runs and the claim can outlive the run: an
empty, create-locked guest sitting on a VMID nobody can reuse.
It cannot be made atomic. The process can die between any two instructions, and a
trap does not survive the SIGKILL exec.CommandContext sends. What was missing is
that the run said nothing: the operator got "Failed to register ...: context
canceled" and no hint that a claim might be sitting on the cluster.
WARNING Aborted while registering VM/CT config 101 (webserver): VMID 101 may be
left reserved and locked on the cluster
Only on a cancellation, and only on the registration arm: applying to a guest that
already exists claims nothing.
The window is two consecutive pmxcfs writes, so it is narrow. It widens exactly
when it matters: pmxcfs blocks when the cluster loses quorum, which is also when
somebody aborts. The finding's reachability stays unproven, and this change does
not pretend otherwise - it costs one conditional and turns an invisible leftover
into a named one.
* feat: judge an all-refused storage against the live definition instead of guessing
When the set schema refuses every staged key, the refusal alone does not say whether
anything was lost. A create-only key may already hold the staged value, in which case
there was nothing to do, or it may differ, in which case the restore could not put it
back. The two are opposite outcomes and the run counted them as one, so a definition
the restore failed to bring back was reported as applied and the summary said
"storage.cfg applied (ok=1 failed=0)".
The guest arm counts the same shape as a failure, which is the other half of the same
inconsistency. Making the count match by fiat would only move the guess: it is right
for one of the two cases and wrong for the other.
The live definition answers it. A pvesh get on the definition, the refused keys
compared against what was staged, and three outcomes instead of one:
INFO Storage definition nfs-backup already matches every staged key the update
schema refuses -> applied
WARNING Failed to apply storage nfs-backup: the update schema refuses server and
the live definition does not match the staged value -> failed
WARNING Applied nothing for storage nfs-backup: the update schema refuses every
staged key (server) and the live definition could not be compared
-> applied
The third arm is the point: a wrong "differs" turns a clean restore into a failure,
which is worse than the silence it replaces, so anything that cannot be compared
keeps the conservative answer. Two shapes the official API returns are excluded for
that reason rather than coerced - a content list is comma-joined in an order PVE does
not promise, and a boolean comes back as 1/0 against a cfg that may spell it either
way.
The read costs one pvesh call, and only for a definition that already had refusals.
The debug lines carry the evidence per key: whether it had a comparable pair at all,
and the two values when it did.
* docs: add this line's restore and legacy findings to the 0.37.0 notes
Four lines and one action, for the parts of this line's work that change what the
user does or reads. The registry's own rule decided what stayed out: the block-form
duplicate, the false "not a known variable" and the enriched debug block are all
corrections to the backup.env check that 0.37.0 itself introduces, so the existing
lines cover them once they are true; the bounded personal-script open and the Go
floor are hardening and build, which the rule excludes by name.
- the legacy category, which none of the three existing lines describes, and which
matters because the old name is the one in use for seven of the twelve;
- a storage definition the restore could not put back, which used to be counted as
applied and now reports;
- a guest configuration written while Proxmox had the guest marked busy;
- the VMID an aborted restore may have left reserved.
The action names no dashboard path because there is none to give: the fix is an edit
to backup.env, and the warning already names the line.
* fix: cap the abandoned personal-script open at one probe per path
personalScriptOpenTimeout bounds the wait, never the open: a blocking open(2)
cannot be cancelled, so the goroutine probeWithin gives up on is abandoned. That
buys the scheduler its life back and is worth one goroutine, but the daemon
reaches this path twice per backup run, pre and post, so a parent that stays
stuck paid it again on every run with nothing counting the total. A goroutine
parked in a syscall holds an OS thread and the runtime aborts the process at
10000 of them.
A path now gets one outstanding probe: while the previous open is still parked,
the script is refused without launching a second. The claim is released from
inside the probe, so the bound lifts itself the moment the open returns, which
is what a recovered NFS or CIFS mount does and that mount is the reachable way
into this state.
Measured before the change: eight calls against a FIFO parent left eight parked
goroutines and three extra OS threads. After it, one.
* test: stand the real ctrl+c error in for the bare one in the what's-new suite
The VERF01 table pinned "any keystroke disarms the warning" with a bare
shell.ErrClosed for ctrl+c. Production never emits that shape. A real ctrl+c
terminates the program, Program.Run wraps the interrupt as ErrProgramKilled, and
the pending Ask resolves through Session.closedErr, which yields
ui: session closed: program was killed: program was interrupted
Nothing failed, because whatsnewRender saves the seen-flag on every non-timeout
error and both shapes land in that branch. It would have failed silently: the day
that rule is narrowed to specific resolutions, the row keeps passing on a value
no operator can produce and the ctrl+c case stops being covered at all.
The value cannot be rebuilt by a caller either. closedErr flattens its cause with
%v rather than %w, so errors.Is(err, tea.ErrInterrupted) is false on what a
caller receives, and any hand-written fixture is a copy of two wordings free to
drift. ClosedByInterrupt therefore builds it by calling the real closedErr, and
the two driven Ask tests pin both halves: a real ctrl+c must equal it, and a
program killed out from under the Ask must not.
Mutation-verified. Dropping closedErr's cause fails the program-died test
("both are ui: session closed"); giving ClosedByInterrupt the wrong cause fails
the ctrl+c test and names the two shapes side by side.
* fix: count a storage definition the restore could not judge as unknown, not applied
The all-refused arm had two buckets for three facts. When every staged key came
back refused and the live definition could not answer, the log said "Applied
nothing for storage X" and the counter said applied++. The caller therefore saw
ok=1 failed=0, applyPVEStorageCfgFromStage returned nil, and a restore whose
storage state is unknown reported success.
Calling it failed instead is no better: nothing there was shown to be wrong, and
failed>0 aborts the staged apply. The shape was the defect, so applyStorageCfg
now returns three counts and the summary lines read ok=N unknown=M failed=K.
unknown does not fail the apply; it stops claiming an apply.
refusedKeysThatDiffer reports coverage rather than a single comparable flag, which
fixes the second half. The flag was raised by the FIRST comparable key, so a
definition with one key compared and one key missing from the live object came
back differing=nil, comparable=true, and the run announced that every refused key
already matched - a check it had made on half of them. It now returns which keys
differ and which it could not judge, the switch orders differing first (a key
proven wrong is a failure even when others went unjudged), and the warning names
the keys it could not compare instead of saying only that the definition could not
be compared.
Debug walks the whole judgement before the warning that summarises it: how many
comparable values the live definition answered with, the verdict per refused key,
then the tally.
Mutation-verified. Counting unknown as applied again fails both new tests;
restoring the first-comparable-key flag fails the partial-coverage one.
* fix: cut the whole hardlink advisory, not the clause that carries its prefix
fb4fc61 moved the sysctl reading out of Reason and normalised what an older
daemon had stored there, so the two sides would stop differing over a fact that
is no longer part of either. It filtered "; "-separated clauses and dropped the
ones starting with fs.protected_hardlinks.
The advisory is not one clause. Disabled reads
fs.protected_hardlinks=0 allows hard-linking root-owned executables; set it to 1
so the split produced two, the filter dropped the half carrying the prefix and
kept "set it to 1", and the stored side came out one clause longer than the live
one. That is PATH STATE CHANGED SINCE STARTUP on a path that did not move - the
exact verdict fb4fc61 exists to stop, still reachable through the one shape that
punctuates itself.
The normaliser now cuts from the prefix to the end of the string, which holds
however the advisory is punctuated. It is safe because there is one producer,
personalScriptForeignAncestorReason, and it appends the advisory last.
The regression test missed it by one character: it put the enforced shape in the
stored Reason, which carries no internal "; ", and the disabled shape in
HardlinkAdvisory, which is never compared. It is now a table over all three
shapes an older daemon could have stored.
Mutation-verified: restoring the clause filter fails the disabled subtest alone.
* fix: register the twelve legacy aliases the table was never derived from
legacyAliases carried seven entries taken from the getStringWithFallback and
getBoolWithFallback call sites, as its own doc comment said. getIntWithFallback,
getStringSliceWithFallback and half the getBoolWithFallback sites were never
read, so eleven names the loader honours reached the operator as
INFO CLOUD_RETENTION_DAYS is not a known variable and is ignored
which is false: the loader reads it whenever MAX_CLOUD_BACKUPS is absent, and an
operator who acts on that line deletes a working setting. Measured before the
change: Unknown 11, Legacy 0. After: Unknown 0, Legacy 11.
A twelfth, AGE_RECIPIENTS, is read by a hand-written fallback rather than a
helper and is registered with a comment saying so.
The eleven all go the same way - the canonical name is consulted first and is the
one the template assigns - so none of them wins, and the renderer's existing INFO
names where to move the value to.
The registry stays hand-written, but it no longer drifts unseen.
TestEveryFallbackNameIsRegisteredOrInTheTemplate walks config.go with go/ast,
takes the key list out of every multi-name lookup, and fails naming the file and
line of any name that is neither registered nor assigned in the template. A
sibling test pins the DIRECTION against the same call sites, because Wins is what
the renderer uses to tell the operator which of the two values is in effect. A
fallback helper this test does not know about fails it too, so the next one
cannot be added quietly. Its one blind spot, the hand-written fallback, is named
in its doc comment rather than papered over.
CONFIGURATION.md called the other pairs "harmless"; they are the safe direction,
not absent, and the check reports them.
Mutation-verified: dropping one entry fails naming that entry and its call site;
flipping one Wins fails naming the call site that contradicts it.
* fix: document in the template the six variables the loader reads and the audit rejected
The audit calls a variable unknown when the template does not mention it, and the
renderer turns that into
INFO PBS_REPOSITORY is not a known variable and is ignored
Six names failed that gate while the loader reads them from backup.env perfectly
well: the three PBS API credentials (config.go:1484-1490, resolved after the
environment and before local auto-detection) and the three Telegram delivery
confirmation settings (config.go:789-791). Both families are documented in
docs/, so the operator finds the variable, sets it, and is then told by the
integrity check that it is ignored.
They are added to the template as COMMENTED examples, which is the rule the audit
already accepts and the shape the webhook stanza uses. Making them active
assignments would have been the wrong repair: every existing backup.env lacks
them, so each would become "absent and falls back to its default" - a WARNING,
counted by ParseLogCounts, promoting a clean run to exit 1 for every operator who
never needed them. The template still assigns 182 variables; only the documented
count moved.
TestEveryNameTheLoaderReadsIsAcceptedByTheAudit walks config.go with go/ast,
collects every literal name passed to a getter, and fails naming the file and
line of any the audit would reject. It is the general form of the check the
legacy-alias test does for aliases alone: no reviewer caught this half, because
the reviewers were looking at the alias table.
Two fixtures move with the template: the debug block pins the template line
numbers CUSTOM_BACKUP_PATHS is assigned on, and the three install
characterization goldens carry the file byte for byte. Their diff is the fourteen
added comment lines and nothing else.
Mutation-verified: removing one commented example fails the test naming that
variable and its call site.
* docs: correct three places that quoted output the binary does not emit
CONFIGURATION.md showed the integrity block as it looked two changes ago: the
debug line reading "multi-value variables skipped", a counts line without the
legacy category, and no line for the template's two halves at all. The sample is
now the real output, captured from the renderer against the shipped template, and
the sentence above it says four findings rather than three because the table below
it has listed four since the legacy category was added.
DAEMON.md promised `RUNNING DAEMON STATE UNAVAILABLE`. No such string exists. The
binary says `Running daemon personal-script state: UNAVAILABLE (<why>)` for the
section and marks each `Daemon now:` line `UNAVAILABLE (<why>)`, and the legend
two lines above already listed the real label - only the prose disagreed.
DEVELOPER_GUIDE.md said Go 1.26+ in both the stack list and the prerequisites.
The enforced floor is 1.26.8: go.mod sets it and goRuntimeMinVersion refuses to
start below it.
Two stale things in the source itself, found while checking the docs against it:
auditRunConfigFile's comment still enumerated three findings, and
integrityVerdictCounts still sized its slice for three parts. Neither changes
behaviour; both are the same drift the docs had.
* fix: show the hardlink advisory in the dashboard and level it by what it says
inspectPersonalScript's own comment states the rule: the mitigation the accepted
foreign-owned ancestor RESTS ON is reported with it, never separately, because an
operator reading "owner can replace descendants" needs to know in the same breath
whether anything is stopping them. The CLI did that.
buildDashboardPersonalScriptLine read Path and Reason and dropped the advisory, so
the screen an operator is most likely to be looking at showed the trust decision
without what stands behind it. It now renders it on its own line under the
verdict, where logPersonalScriptDiagnostic puts it.
The second half was not reported by any reviewer. The advisory says two opposite
things and both were logged at WARNING:
WARNING Configuration: fs.protected_hardlinks=1 blocks hard-linking root-owned executables
That is the reassurance that the protection IS in force, raised as an alarm among
lines that all mean the opposite. It does not touch the exit code -
runDaemonStatus derives that from diagnostics.Level, not from counting warnings -
but it tells the operator to act on something already right. Off or unreadable
stays a WARNING; in force is now INFO, and Subtle rather than WarningText on the
dashboard.
The level is decided from a fact, not from the sentence.
personalScriptHardlinkAdvisory returns the reading alongside its text and
personalScriptDiagnostic carries it as HardlinkProtectionInForce, so a renderer
never matches on words that an edit to the message could change under it. The two
halves are written together and a test pins that they cannot disagree, including
for a value the message was not written around.
Mutation-verified: removing the dashboard block fails the parity test; making the
CLI unconditionally WARNING fails the enforced case alone.
* docs: cover this line's configuration and storage work in the 0.37.0 notes
Two facts from this line were not on Screen 0.
The storage line said "reports a storage definition it could not put back",
which was the failed case alone. The apply now has three outcomes and the third
is the one that used to be silent, so the line says what the count means: applied
only when the restore can show it, otherwise failed or unknown.
The new line is the one with a consequence. Nineteen names the loader honours -
twelve legacy aliases and seven variables the template never mentioned - were
reported as "not a known variable and is ignored", and an operator who acted on
that deleted working configuration. It gets the eighth and last highlight slot,
next to the other configuration-check lines rather than between the two restore
ones.
The matching action tells anyone who saw that line in an earlier version to check
before deleting, because the advice was wrong and the file may already have been
edited on it.
* fix: mark the release notes seen only when a person closed Screen 0
whatsnewRender saved the seen-flag for every error that was not a timeout or a
torn-down parent. shell.Ask returns ErrClosed for a UI that DIED as well as for
ctrl+c, so a renderer or terminal failure marked the notes seen for an operator
who never got to read them, permanently: the version does not change again, so
the screen never reopens.
Telling the two apart was impossible for a caller. Session.closedErr flattened
the program's termination error with %v, so errors.Is(err, tea.ErrInterrupted)
was false on the value a caller actually receives, and only the wording separated
"program was killed: program was interrupted" from "program was killed: <any
other cause>". closedErr now wraps with %w and the interrupt survives in the
chain. The change is additive - it adds links and removes none - so every
errors.Is(err, ErrClosed) keeps matching exactly as before; verified that every
consumer of an Ask error goes through shell.IsAbort, and that the
context.Canceled checks elsewhere in the tree read errors from internal/input and
from command timeouts, never a closedErr.
shell.IsUserInterrupt owns the encoding of "a person did this", so the caller
asks one question without importing bubbletea. whatsnewRender withholds the write
for a closed session that is not one.
This does NOT re-arm issue #305. Esc and q never reach that arm - the pager
resolves them itself, as nil or as its own abort sentinel - and ctrl+c still
writes. What stops writing is the UI going away on its own.
Two fixtures were the bare ErrClosed production never emits for ctrl+c, the same
defect already corrected in the VERF01 suite. Both now use the real value, and
both tables gained the cases the old contract could not express: a UI that died,
and a session that closed with no error at all (something else quit the program
while the Ask was pending, so nobody resolved the screen).
Mutation-verified: restoring %v fails the chain assertion and the ctrl+c write;
dropping the new arm fails the UI-death case in both suites.
* docs: define the unknown storage outcome the summary now reports
The SAFE Apply transcripts were updated to show ok=N unknown=M failed=K when
applyStorageCfg grew its third count, but nothing in the guide said what unknown
means, so the sample introduced a number with no definition behind it.
The detailed section now carries the three outcomes side by side, what puts a
definition in unknown (the update schema refuses every staged key and the live
definition could not be compared: pvesh get failed, or a refused key is absent
from the live object or is not a plain scalar), and the line the run prints when
it happens.
It also states the part an operator has to act on: unknown does not stop the
restore, only failed > 0 does, so a restore that ended without an error is not
the same as a restore that is complete. Those definitions have to be checked by
hand against the export.
Reported by CodeRabbit on the transcript at line 728; verified against
pve_staged_apply.go, which returns an error only for failed > 0, and against the
unknown arm in restore_cluster_apply.go.
* docs: correct the claim that a failed storage apply stops the restore
The paragraph added one commit ago said "only failed > 0 stops the restore". It
does not, in either path, and the section sits under the SAFE cluster apply
transcript where it is furthest from true.
maybeApplyStorageCfg logs the summary and returns nil unconditionally; even the
context error it may get back from applyStorageCfg becomes a Warning. The flow
goes on to datacenter.cfg with no record beyond the log lines.
The staged path does return an error for failed > 0, but the caller does not stop
either: runStageApplyStep halts only for a user abort or
ErrRestoreInconsistentState, and turns anything else into restoreHadWarnings plus
a warning line, then keeps applying the remaining categories. So the difference
between the two paths is not stop versus continue, it is what the outcome
records, and the paragraph now says that instead.
The operator-facing consequence is unchanged and is the part that matters: a
restore that ended without an error is not a restore that is complete, and every
definition the run named has to be checked by hand.
Reported by Greptile against the SAFE apply flow. Their point was right and the
correction is wider than the one they described: they read the staged path as
stopping, and it does not either.
---------
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bumps the minor-updates group with 1 update in the / directory: golang.org/x/crypto.
Updates
golang.org/x/cryptofrom 0.55.0 to 0.56.0Commits
86efde5ssh: reject unexpected message types on established channelsa6cdac6ssh: drop traffic on undecided channels39dc44essh: don't skip the source-address critical option in CheckCertafebf4cx509roots/fallback/bundle: make subjectsEqual stricter on Go 1.27+89f4e9bx509roots/fallback: update bundle71488c4ssh/knownhosts: compare only public key portions for revocation82adefassh: synchronize unexpected response testc757c98all: upgrade go directive to at least 1.26.0 [generated]593c81assh: correctly ignore pre-banner lines46efc8bacme: add crypto.SignMessage test coverage