mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-09-10 02:25:56 +00:00
Merge pull request #1964 from rcourtman/maintainer/20260907T181011Z-release-v6.4
Protect v6.4 alert credentials and prevent stale discovery rollback
This commit is contained in:
@@ -13,6 +13,14 @@ on:
|
||||
description: 'Exact 40-character commit SHA admitted for this release'
|
||||
required: true
|
||||
type: string
|
||||
release_source_branch:
|
||||
description: 'Governed source branch for a merged immutable release snapshot'
|
||||
required: false
|
||||
type: string
|
||||
release_pull_request:
|
||||
description: 'Merged pull request that reviewed the immutable snapshot'
|
||||
required: false
|
||||
type: string
|
||||
release_notes:
|
||||
description: 'Release notes (markdown)'
|
||||
required: true
|
||||
@@ -85,6 +93,10 @@ permissions:
|
||||
jobs:
|
||||
# Combined version extraction and validation (saves a checkout)
|
||||
prepare:
|
||||
permissions:
|
||||
actions: read
|
||||
contents: read
|
||||
pull-requests: read
|
||||
# Stable releases use hosted runners regardless of their Windows-signing
|
||||
# decision. Prereleases retain the credential-free PVE acceleration path.
|
||||
runs-on: ${{ !contains(inputs.version, '-') && 'ubuntu-24.04' || fromJSON('["self-hosted","Linux","X64","pulse-pve-compile"]') }}
|
||||
@@ -134,11 +146,20 @@ jobs:
|
||||
persist-credentials: false
|
||||
fetch-depth: 0
|
||||
|
||||
- name: Verify reviewed release snapshot
|
||||
id: snapshot
|
||||
env:
|
||||
GH_TOKEN: ${{ github.token }}
|
||||
RELEASE_SOURCE_BRANCH: ${{ inputs.release_source_branch }}
|
||||
RELEASE_PULL_REQUEST: ${{ inputs.release_pull_request }}
|
||||
run: python3 scripts/release_control/release_snapshot.py
|
||||
|
||||
- name: Extract version
|
||||
id: extract
|
||||
env:
|
||||
VERSION_INPUT: ${{ inputs.version }}
|
||||
HISTORICAL_ASSET_BACKFILL_INPUT: ${{ inputs.historical_asset_backfill_only }}
|
||||
SNAPSHOT_SOURCE_BRANCH: ${{ steps.snapshot.outputs.source_branch }}
|
||||
run: |
|
||||
set -euo pipefail
|
||||
if [[ ! "${VERSION_INPUT}" =~ ^[0-9]+\.[0-9]+\.[0-9]+(-((rc|alpha|beta)\.[0-9]+))?$ ]]; then
|
||||
@@ -164,12 +185,12 @@ jobs:
|
||||
exit 1
|
||||
fi
|
||||
|
||||
SOURCE_BRANCH="${GITHUB_REF_NAME}"
|
||||
SOURCE_BRANCH="${SNAPSHOT_SOURCE_BRANCH}"
|
||||
HISTORICAL_ASSET_BACKFILL_ONLY="${HISTORICAL_ASSET_BACKFILL_INPUT}"
|
||||
python3 scripts/write_github_output.py tag "${TAG}"
|
||||
python3 scripts/write_github_output.py version "${VERSION}"
|
||||
echo "is_prerelease=${IS_PRERELEASE}" >> $GITHUB_OUTPUT
|
||||
echo "source_branch=${SOURCE_BRANCH}" >> $GITHUB_OUTPUT
|
||||
python3 scripts/write_github_output.py source_branch "${SOURCE_BRANCH}"
|
||||
python3 scripts/write_github_output.py historical_asset_backfill_only "${HISTORICAL_ASSET_BACKFILL_ONLY}"
|
||||
echo "Version: ${VERSION}, Tag: ${TAG}, Prerelease: ${IS_PRERELEASE}, Branch: ${SOURCE_BRANCH}, HistoricalBackfillOnly: ${HISTORICAL_ASSET_BACKFILL_ONLY}"
|
||||
|
||||
|
||||
@@ -230,6 +230,8 @@ scripts/release_control/*
|
||||
!scripts/release_control/contract_audit_test.py
|
||||
!scripts/release_control/customer_promotion_lease.sh
|
||||
!scripts/release_control/control_plane.py
|
||||
!scripts/release_control/release_snapshot.py
|
||||
!scripts/release_control/release_snapshot_test.py
|
||||
!scripts/release_control/generate_platform_support_frontend_module.py
|
||||
!scripts/release_control/control_plane_audit.py
|
||||
!scripts/release_control/control_plane_audit_test.py
|
||||
|
||||
@@ -0,0 +1,23 @@
|
||||
# Discord diagnostic credential backport
|
||||
|
||||
Security backport of Core commit 3c77ecb338ee3d68f1dd7d57a367b4727040a61c,
|
||||
limited to the notification redactor and its regression tests. Eligible under
|
||||
Release Train rules 2/4: synthetic tests on release-line base
|
||||
acb841d3d667545a493033fad48a810db84586b5 exposed Discord webhook path tokens.
|
||||
Discord documents secure webhook tokens and token-authorised operations:
|
||||
https://docs.discord.com/developers/resources/webhook (read 7 September 2026).
|
||||
|
||||
The change masks the suffix after /webhooks/ on exact Discord hosts, including
|
||||
legacy/versioned paths and escaped credentials. It modifies diagnostics, not
|
||||
request destinations, and preserves transport error causes.
|
||||
|
||||
Validation: importing tests alone reproduced failures in helper output and
|
||||
transport diagnostics. With the repair, focused race tests repeated 20 times
|
||||
passed, covering helper cases, transport errors and actual rate-limit logs.
|
||||
Logs are retained in the lane packet 20260907T170524Z-release-line.
|
||||
No full suite, release qualification or installed delivery test was performed.
|
||||
|
||||
This does not scrub historical delivery text or establish customer exposure.
|
||||
Telegram parsing review remains separate. Protected PR checks, steward risk
|
||||
assessment and exact-candidate qualification remain required; prior adverse
|
||||
latency and excluded crash evidence are not cleared by these focused tests.
|
||||
@@ -0,0 +1,32 @@
|
||||
# Encoded-query and resolved-ntfy diagnostic security backport
|
||||
|
||||
Release Train rules 2/4: named security defects reproduced on supplied candidate
|
||||
71c721ccee8bea8a64a130fdbc0ea78292ff748b, not new product scope.
|
||||
Source: Core 21ed7a89270a5ee1850254d0289c013567871fa8.
|
||||
Only that correction is backported; mixed main is not imported. The HTTP test
|
||||
context was absent here, so retain the complete diagnostic-context test from
|
||||
that source when resolving the conflict. Production HTTP code is unchanged.
|
||||
|
||||
Before repair, TestDeliveryEncodedQueryConfidentiality and
|
||||
TestDeliveryResolvedNtfyConfidentiality fail using synthetic credentials and
|
||||
in-memory transports. After repair, the focused notification/redaction tests
|
||||
and TestGetDeliveryLog HTTP tests pass. Logs are retained in
|
||||
/var/lib/pulse-maintainer/queue/staging/20260907T181017Z-release-line/
|
||||
(before.log, after.log, api.log; race.log records the separate repeated matrix).
|
||||
|
||||
The repair decodes supported query names once and masks repeated occurrences;
|
||||
resolved ntfy projects transport errors before logging and returning. The
|
||||
matrix checks unchanged destination, payload, event identity, safe userinfo
|
||||
rejection and error-cause unwrapping. This is bounded diagnostic protection,
|
||||
not arbitrary-secret detection, evidence of customer exposure, recipient
|
||||
receipt, full-suite qualification or release approval.
|
||||
|
||||
Fresh external acceptance context: OWASP Logging Cheat Sheet, Data to exclude
|
||||
and Verification, inspected 2026-09-07:
|
||||
https://cheatsheetseries.owasp.org/cheatsheets/Logging_Cheat_Sheet.html
|
||||
Credentials should not be logged; diagnostic failure paths need verification.
|
||||
|
||||
Delivery still owns protected-PR integration and exact-candidate qualification.
|
||||
The retained 940f788d latency failure is not cleared by these focused tests;
|
||||
Benchmarks remains advisory for source landing. Stable needs exact RC soak
|
||||
and founder packet approval. No excluded crash investigation was performed.
|
||||
@@ -0,0 +1,25 @@
|
||||
# Telegram diagnostic credential backport — 7 September 2026
|
||||
|
||||
Release Train rules 2/4 permit this named security repair on base
|
||||
`dcf7e499613679c941abe49800f1294b5714fde1`. Backports only code, tests and
|
||||
notification contract from reviewed Core `192a72e05c06b2a2ff3a04bb2ef53ded78950e47`.
|
||||
Existing Discord masking and diagnostic context handling are preserved.
|
||||
|
||||
Fresh independent primary evidence: https://core.telegram.org/bots/api#making-requests
|
||||
identifies bot authentication tokens in request paths and supports local API
|
||||
servers. This is diagnostic containment, not a new product surface.
|
||||
|
||||
Tests imported alone failed (before.log, exit 1):
|
||||
`go test ./internal/notifications -run '^Test(RedactWebhookURLSecrets|TelegramWebhookDiagnosticsRedactPath)$' -count=1`.
|
||||
Synthetic escaped path credentials survived helper and transport diagnostics;
|
||||
non-credential bot hostnames/query URLs also lost diagnostic context.
|
||||
|
||||
Repair validation (after.log):
|
||||
`go test -race ./internal/notifications -run '^Test(RedactWebhook|WebhookRateLimitLogsRedactURLSecrets|SlackWebhookDiagnosticsRedactPath|DiscordWebhookDiagnosticsRedactPath|TelegramWebhookDiagnosticsRedactPath)' -count=20`.
|
||||
|
||||
No full qualification, installed change, customer exposure assertion, historical
|
||||
stored-data cleanup or recipient receipt proof. Protected integration and all
|
||||
eight enforced checks remain required. Existing adverse latency and excluded
|
||||
crash evidence are not cleared. Delivery owns release maturity and publication.
|
||||
|
||||
Result: exit 0, all selected tests passed with race detection (20 repeats).
|
||||
@@ -0,0 +1 @@
|
||||
ok github.com/rcourtman/pulse-go-rewrite/internal/notifications 1.142s
|
||||
@@ -0,0 +1,18 @@
|
||||
--- FAIL: TestRedactWebhookURLSecrets (0.00s)
|
||||
--- FAIL: TestRedactWebhookURLSecrets/telegram_escaped_prefix (0.00s)
|
||||
webhook_url_redaction_test.go:76: RedactWebhookURLSecrets() = "https://api.telegram.org/%62ot123:telegram-secret/sendMessage", want "https://api.telegram.org/botREDACTED/sendMessage"
|
||||
--- FAIL: TestRedactWebhookURLSecrets/telegram_local_escaped_prefix (0.00s)
|
||||
webhook_url_redaction_test.go:76: RedactWebhookURLSecrets() = "http://localhost:8081/%62ot123:telegram-secret/sendMessage", want "http://localhost:8081/botREDACTED/sendMessage"
|
||||
--- FAIL: TestRedactWebhookURLSecrets/telegram_no_method_with_URL_query (0.00s)
|
||||
webhook_url_redaction_test.go:76: RedactWebhookURLSecrets() = "https://api.telegram.org/botREDACTED//example.org/status", want "https://api.telegram.org/botREDACTED?next=https://example.org/status"
|
||||
--- FAIL: TestRedactWebhookURLSecrets/bot_query_is_not_a_path (0.00s)
|
||||
webhook_url_redaction_test.go:76: RedactWebhookURLSecrets() = "https://example.org/hook?next=https://botREDACTED/status", want "https://example.org/hook?next=https://bot.example.org/status"
|
||||
--- FAIL: TestRedactWebhookURLSecrets/telegram_fragment (0.00s)
|
||||
webhook_url_redaction_test.go:76: RedactWebhookURLSecrets() = "https://api.telegram.org/botREDACTED", want "https://api.telegram.org/botREDACTED#diagnostic"
|
||||
--- FAIL: TestRedactWebhookURLSecrets/bot_hostname_is_not_a_path (0.00s)
|
||||
webhook_url_redaction_test.go:76: RedactWebhookURLSecrets() = "https://botREDACTED/hook?channel=ops", want "https://bot.example.org/hook?channel=ops"
|
||||
--- FAIL: TestTelegramWebhookDiagnosticsRedactPath (0.00s)
|
||||
webhook_url_redaction_test.go:228: unsafe transport diagnostic: Post "https://api.telegram.org/%62ot123:telegram-secret/sendMessage": connection refused
|
||||
FAIL
|
||||
FAIL github.com/rcourtman/pulse-go-rewrite/internal/notifications 0.005s
|
||||
FAIL
|
||||
@@ -254,8 +254,19 @@ without the other lanes changing the candidate underneath it.
|
||||
every third train and records the decision here.
|
||||
2. Each train has its own branch, `release/v6.N`, created from `main` at cut
|
||||
time and declared in `docs/release-control/control_plane.json` so the
|
||||
release workflow refuses a dispatch from any other branch. `main` is never
|
||||
frozen. A fix for something found in the candidate is backported to the
|
||||
release workflow verifies that governed source line. `main` is never
|
||||
frozen. A selected release is an immutable commit, not the current tip of
|
||||
the train. Its preparation PR uses a fixed `release-candidate/<packet>`
|
||||
ref and passes the normal protected review path into the governed line.
|
||||
Qualification, builds and publication use that PR's exact head commit,
|
||||
even when the merge or later train commits contain newer work. Snapshot
|
||||
dispatch must verify the merged PR's head, canonical repository, source
|
||||
line and continued ancestry in published history. The workflow and source
|
||||
SHA must both equal the admitted snapshot. Later commits belong to another
|
||||
release unless the maintainer explicitly rejects the selected candidate
|
||||
for a concrete defect in that candidate. Merely finding newer work does
|
||||
not invalidate qualification or restart a release.
|
||||
A fix for something found in the candidate is backported to the
|
||||
release branch through a pull request; each backport produces the next
|
||||
`rc.N` and restarts the soak. After general availability the branch is
|
||||
the patch line for that train.
|
||||
|
||||
@@ -15,6 +15,25 @@
|
||||
|
||||
## Purpose
|
||||
|
||||
### Immutable release source
|
||||
|
||||
Continuous development must not change an admitted release's source. The
|
||||
preparation PR's qualified head stays fixed on `release-candidate/<packet>`
|
||||
while its governed source branch continues receiving work. Dispatch verifies
|
||||
that the canonical PR merged into the version's governed source line, that its
|
||||
head and ref match the admitted snapshot, and that its merge remains in that
|
||||
line's published history. Qualification, workflow execution, compiler dispatch
|
||||
and published artifacts bind to that head, not a later merge or branch tip.
|
||||
The source workflow must implement the snapshot input and provenance contract
|
||||
before the maintainer spends an exact qualification run on it. Later changes
|
||||
belong to the next candidate unless the maintainer explicitly rejects the
|
||||
selected source for a concrete defect. Existing maturity, soak, failed-check
|
||||
and publication-authority boundaries still apply.
|
||||
`scripts/release_control/release_snapshot.py` owns snapshot identity validation.
|
||||
Its executable identity cases are in `release_snapshot_test.py`, and the staged
|
||||
workflow contract is verified in `release_promotion_policy_test.py`.
|
||||
|
||||
|
||||
### Benchmark qualification evidence
|
||||
|
||||
The Build and Test benchmark job retains `bench-metadata.txt` together with
|
||||
|
||||
@@ -17,6 +17,23 @@
|
||||
|
||||
## Purpose
|
||||
|
||||
**Availability backfill preserves concurrent discovery changes (7 September 2026)**
|
||||
|
||||
The backfill List snapshot is a work list, not an authoritative record to save.
|
||||
Read the current discovery, derive a missing suggestion from its current identity,
|
||||
and persist under one store write lock. State-provider reads stay outside that
|
||||
lock. Preserve newer identity, URL, engine version, user notes and existing
|
||||
(including dismissed) proposals; never resurrect a discovery deleted after List.
|
||||
A failed persistence attempt remains an error without installing the proposed
|
||||
change in cache. `TestService_BackfillPreservesConcurrentManualRepair` in `service_test.go`
|
||||
deterministically pauses SetReadState
|
||||
backfill after List, completes manual ESPHome repair, then resumes backfill and
|
||||
checks both cache and encrypted restart.
|
||||
`TestStore_BackfillAvailabilitySuggestionUsesCurrentRecord` in `store_test.go`
|
||||
covers current-identity
|
||||
inference, dismissal, deletion, unsupported identity and persistence failure.
|
||||
This is discovery state-integrity proof, not incident or notification acceptance.
|
||||
|
||||
### Host-local network evidence exclusion
|
||||
|
||||
Automatic PVE association must not treat loopback, unspecified, multicast or
|
||||
|
||||
@@ -607,3 +607,57 @@ Queue-free regression tests cover both hosts, encoded and legacy paths,
|
||||
lookalike/unrelated hosts, transport errors and actual rate-limit log output.
|
||||
This does not establish customer exposure, recipient receipt or recognition of
|
||||
arbitrary custom webhook secrets.
|
||||
|
||||
### Discord webhook diagnostic path confidentiality
|
||||
|
||||
On exact `discord.com` and legacy `discordapp.com` hosts, the shared redactor
|
||||
masks the suffix after `/webhooks/`, including the webhook ID and token.
|
||||
Versioned API prefixes remain visible; encoded paths, host casing and ports
|
||||
cannot bypass masking. Unrelated paths and lookalike hosts are unchanged.
|
||||
Configured destinations and transport error causes are not modified.
|
||||
|
||||
[Discord's webhook reference](https://docs.discord.com/developers/resources/webhook)
|
||||
identifies the secure webhook token and token-authorised operations. Focused
|
||||
synthetic regressions cover helper output, transport diagnostics and actual
|
||||
rate-limit logs. This is not evidence of customer exposure, recipient receipt,
|
||||
release qualification, or protection of arbitrary custom-host credentials.
|
||||
|
||||
### Telegram diagnostic path parsing
|
||||
|
||||
Telegram bot-path masking operates on the parsed, decoded URL path and clears
|
||||
RawPath after replacement. This covers percent-encoded bot prefixes without
|
||||
mistaking a hostname or a URL inside a query for a bot path. Method suffixes,
|
||||
query diagnostics and fragments remain intact; configured destinations and
|
||||
transport-error causes are unchanged. Host-independent masking is retained for
|
||||
local API servers, which are supported by the
|
||||
[Telegram API documentation](https://core.telegram.org/bots/api#making-requests).
|
||||
|
||||
Focused regression tests cover escaped prefixes/tokens, local servers, missing
|
||||
method suffixes, query URLs, fragments, transport errors and rate-limit logs.
|
||||
This is diagnostic containment, not evidence of customer exposure or recipient
|
||||
delivery. Arbitrary path secrets and unrecognised query credentials remain
|
||||
outside this bounded change.
|
||||
|
||||
### Bounded diagnostic confidentiality: query representations and bypass callers
|
||||
|
||||
Recognised query names are exactly token, apikey, api_key, key, secret and
|
||||
password after one URL query decode. Every repeated occurrence is masked,
|
||||
including mixed literal/escaped names. Unrelated names, ordering and values
|
||||
remain intact; invalid name escapes fail closed. This is diagnostic projection,
|
||||
not mutation of configured destinations or a claim to recognise arbitrary secrets.
|
||||
|
||||
Resolved ntfy must apply the same transport-error projection before both its
|
||||
error log and returned error. Common HTTP execution preserves payload bytes,
|
||||
event identity and error causes; URLs containing userinfo remain rejected by
|
||||
outbound validation even though historical diagnostic userinfo is masked.
|
||||
|
||||
The caller matrix and retained Delivery regression tests exercise URL/message
|
||||
helpers, actual rate-limit logs, common transport and resolved-ntfy transport
|
||||
errors/logs with synthetic secrets. HTTP delivery-log regression verifies encoded
|
||||
and repeated query credentials while retaining diagnostic context and entry
|
||||
identity. Existing exact-output tables bound Slack/GovSlack/legacy, Discord,
|
||||
Telegram/local paths, malformed URLs and non-secret lookalikes. Earlier proof
|
||||
missed decoded query representations and a separate ntfy transport caller:
|
||||
provider-only helper examples were not sufficient sink coverage. This contract
|
||||
does not assert arbitrary response-body/third-party error secrecy, installed
|
||||
recipient delivery, candidate qualification or historical customer exposure.
|
||||
|
||||
@@ -329,3 +329,39 @@ func containsAny(value string, needles ...string) bool {
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// These are response-boundary checks, not just URL-helper checks.
|
||||
func TestGetDeliveryLogDiagnosticContext(t *testing.T) {
|
||||
for _, tc := range []struct{ name, input, want string }{
|
||||
{"plain", "connection refused", "connection refused"},
|
||||
{"encoded repeated query", "Post https://example.test/hook?%74oken=secret&token=secret&channel=ops failed", "Post https://example.test/hook?%74oken=REDACTED&token=REDACTED&channel=ops failed"},
|
||||
{"userinfo", "Post https://user:password@example.test/hook: timeout", "Post https://REDACTED@example.test/hook: timeout"},
|
||||
{"malformed", "Post https://user:password@example.test/%zz: timeout", "[invalid webhook URL]"},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
manager := new(MockNotificationManager)
|
||||
monitor := new(MockNotificationMonitor)
|
||||
monitor.On("GetNotificationManager").Return(manager).Once()
|
||||
manager.On("GetDeliveryLog", mock.Anything, 0).Return([]notifications.DeliveryLogEntry{
|
||||
{NotificationID: "attempt-1", ErrorMessage: tc.input, FailureClass: "transport"},
|
||||
}, nil).Once()
|
||||
rec := httptest.NewRecorder()
|
||||
NewNotificationHandlers(nil, monitor).GetDeliveryLog(rec, httptest.NewRequest(http.MethodGet, "/api/notifications/delivery-log", nil))
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d", rec.Code)
|
||||
}
|
||||
var response struct {
|
||||
Entries []notifications.DeliveryLogEntry `json:"entries"`
|
||||
}
|
||||
if err := json.Unmarshal(rec.Body.Bytes(), &response); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(response.Entries) != 1 || response.Entries[0].ErrorMessage != tc.want ||
|
||||
response.Entries[0].NotificationID != "attempt-1" || response.Entries[0].FailureClass != "transport" {
|
||||
t.Fatalf("unexpected delivery projection: %#v", response)
|
||||
}
|
||||
manager.AssertExpectations(t)
|
||||
monitor.AssertExpectations(t)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2776,6 +2776,7 @@ func (n *NotificationManager) sendResolvedWebhookNtfy(webhook WebhookConfig, ale
|
||||
|
||||
resp, err := n.webhookClient.Do(req)
|
||||
if err != nil {
|
||||
err = redactWebhookTransportError(err)
|
||||
log.Error().
|
||||
Err(err).
|
||||
Str("webhook", webhook.Name).
|
||||
|
||||
@@ -0,0 +1,158 @@
|
||||
package notifications
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"errors"
|
||||
"fmt"
|
||||
"github.com/rs/zerolog"
|
||||
"github.com/rs/zerolog/log"
|
||||
"io"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
// Bounded Delivery assessment. All credentials are synthetic; no network sends.
|
||||
func TestDeliveryEncodedQueryConfidentiality(t *testing.T) {
|
||||
for _, key := range []string{"token", "apikey", "api_key", "key", "secret", "password"} {
|
||||
t.Run(key, func(t *testing.T) {
|
||||
encoded := "%" + "74" + key[1:]
|
||||
// Encode the first byte of each already-supported query key.
|
||||
switch key[0] {
|
||||
case 'a':
|
||||
encoded = "%61" + key[1:]
|
||||
case 'k':
|
||||
encoded = "%6b" + key[1:]
|
||||
case 's':
|
||||
encoded = "%73" + key[1:]
|
||||
case 'p':
|
||||
encoded = "%70" + key[1:]
|
||||
}
|
||||
target := "https://example.test/hook?" + encoded + "=delivery-fixture-secret&channel=ops"
|
||||
parsed, err := url.Parse(target)
|
||||
if err != nil || parsed.Query().Get(key) != "delivery-fixture-secret" {
|
||||
t.Fatal("invalid fixture")
|
||||
}
|
||||
if strings.Contains(RedactWebhookURLSecrets(target), "delivery-fixture-secret") {
|
||||
t.Error("URL helper exposes encoded-key credential")
|
||||
}
|
||||
if strings.Contains(RedactWebhookDiagnosticSecrets("Post "+target+" failed"), "delivery-fixture-secret") {
|
||||
t.Error("diagnostic helper exposes encoded-key credential")
|
||||
}
|
||||
var captured bytes.Buffer
|
||||
original := log.Logger
|
||||
log.Logger = zerolog.New(&captured)
|
||||
defer func() { log.Logger = original }()
|
||||
manager := &NotificationManager{webhookRateLimits: make(map[string]*webhookRateLimit)}
|
||||
for range WebhookRateLimitMax + 2 {
|
||||
manager.checkWebhookRateLimit(target)
|
||||
}
|
||||
if !strings.Contains(captured.String(), "rate limit exceeded") {
|
||||
t.Fatal("log path not reached")
|
||||
}
|
||||
if strings.Contains(captured.String(), "delivery-fixture-secret") {
|
||||
t.Error("rate-limit log exposes encoded-key credential")
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
type deliveryFailTransport struct{ cause error }
|
||||
|
||||
func (d deliveryFailTransport) RoundTrip(*http.Request) (*http.Response, error) { return nil, d.cause }
|
||||
func TestDeliveryResolvedNtfyConfidentiality(t *testing.T) {
|
||||
for _, key := range []string{"token", "apikey", "api_key", "key", "secret", "password"} {
|
||||
t.Run(key, func(t *testing.T) { testResolvedNtfyConfidentiality(t, key) })
|
||||
}
|
||||
}
|
||||
func testResolvedNtfyConfidentiality(t *testing.T, key string) {
|
||||
manager := &NotificationManager{webhookRateLimits: make(map[string]*webhookRateLimit)}
|
||||
if err := manager.UpdateAllowedPrivateCIDRs("127.0.0.1/32"); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
cause := errors.New("synthetic transport failure")
|
||||
manager.webhookClient = &http.Client{Transport: deliveryFailTransport{cause}}
|
||||
var captured bytes.Buffer
|
||||
original := log.Logger
|
||||
log.Logger = zerolog.New(&captured)
|
||||
defer func() { log.Logger = original }()
|
||||
err := manager.sendResolvedWebhookNtfy(WebhookConfig{Name: "fixture", Service: "ntfy", URL: fmt.Sprintf("http://127.0.0.1/topic?%s=delivery-fixture-secret&%%%02x%s=delivery-fixture-secret", key, key[0], key[1:])}, nil, time.Now())
|
||||
if !errors.Is(err, cause) {
|
||||
t.Fatalf("transport not reached: %v", err)
|
||||
}
|
||||
if strings.Contains(err.Error(), "delivery-fixture-secret") {
|
||||
t.Error("resolved ntfy transport error exposes literal token credential")
|
||||
}
|
||||
if !strings.Contains(captured.String(), "failed to send resolved ntfy webhook") {
|
||||
t.Fatal("error log not reached")
|
||||
}
|
||||
if strings.Contains(captured.String(), "delivery-fixture-secret") {
|
||||
t.Error("resolved ntfy error log exposes literal token credential")
|
||||
}
|
||||
}
|
||||
|
||||
// A finite cross-sink matrix: no network sends or persistent queue.
|
||||
func TestWebhookConfidentialityCallerMatrix(t *testing.T) {
|
||||
targets := []string{
|
||||
"https://fixture-user:fixture-secret@example.test/hook",
|
||||
"https://hooks.slack.com/services/team/id/fixture-secret",
|
||||
"https://hooks.slack-gov.com/legacy/fixture-secret",
|
||||
"https://discord.com/api/v10/webhooks/id/fixture-secret",
|
||||
"https://discordapp.com/api/webhooks/id/fixture-secret",
|
||||
"https://api.telegram.org/%62otfixture-secret/sendMessage",
|
||||
"http://127.0.0.1:8081/botfixture-secret/sendMessage",
|
||||
}
|
||||
for _, key := range []string{"token", "apikey", "api_key", "key", "secret", "password"} {
|
||||
encoded := fmt.Sprintf("%%%02x%s", key[0], key[1:])
|
||||
targets = append(targets, "https://example.test/hook?"+key+"=fixture-secret&"+encoded+"=fixture-secret&channel=ops")
|
||||
}
|
||||
for _, target := range targets {
|
||||
t.Run(target, func(t *testing.T) {
|
||||
want := RedactWebhookURLSecrets(target)
|
||||
if strings.Contains(want, "fixture-secret") || strings.Contains(want, "fixture-user") {
|
||||
t.Fatal("unsafe URL")
|
||||
}
|
||||
if got := RedactWebhookDiagnosticSecrets("Post " + target + " failed"); got != "Post "+want+" failed" {
|
||||
t.Fatalf("context lost: %s", got)
|
||||
}
|
||||
var captured bytes.Buffer
|
||||
original := log.Logger
|
||||
log.Logger = zerolog.New(&captured)
|
||||
defer func() { log.Logger = original }()
|
||||
manager := &NotificationManager{webhookRateLimits: make(map[string]*webhookRateLimit)}
|
||||
for range WebhookRateLimitMax + 2 {
|
||||
manager.checkWebhookRateLimit(target)
|
||||
}
|
||||
if strings.Contains(captured.String(), "fixture-secret") || !strings.Contains(captured.String(), "rate limit exceeded") {
|
||||
t.Fatal("unsafe/missing rate-limit log")
|
||||
}
|
||||
cause := errors.New("synthetic transport failure")
|
||||
payload := []byte(`{"event":"unchanged"}`)
|
||||
sent := false
|
||||
manager.webhookClient = &http.Client{Transport: confidentialityTransport(func(req *http.Request) (*http.Response, error) {
|
||||
sent = true
|
||||
body, err := io.ReadAll(req.Body)
|
||||
if err != nil || !bytes.Equal(body, payload) || req.URL.String() != target || req.Header.Get("X-Pulse-Event-ID") != "event-1" {
|
||||
t.Error("request identity changed")
|
||||
}
|
||||
return nil, cause
|
||||
})}
|
||||
_, err := manager.executeWebhookRequest(WebhookConfig{URL: target}, payload, webhookRequestOptions{eventID: "event-1"})
|
||||
if strings.Contains(target, "fixture-user") {
|
||||
if sent || err == nil || !strings.Contains(err.Error(), "URL userinfo is not allowed") || strings.Contains(err.Error(), "fixture-secret") {
|
||||
t.Fatalf("userinfo must be safely rejected: %v", err)
|
||||
}
|
||||
return
|
||||
}
|
||||
if !sent || !errors.Is(err, cause) || strings.Contains(err.Error(), "fixture-secret") || !strings.Contains(err.Error(), "synthetic transport failure") {
|
||||
t.Fatalf("unsafe/missing transport failure: %v", err)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
type confidentialityTransport func(*http.Request) (*http.Response, error)
|
||||
|
||||
func (f confidentialityTransport) RoundTrip(req *http.Request) (*http.Response, error) { return f(req) }
|
||||
@@ -35,52 +35,54 @@ func RedactWebhookURLSecrets(urlString string) string {
|
||||
}
|
||||
parsed.RawPath = ""
|
||||
urlString = parsed.String()
|
||||
}
|
||||
|
||||
// Telegram bot credentials are path components rather than query values.
|
||||
if idx := strings.Index(urlString, "/bot"); idx != -1 {
|
||||
if endIdx := strings.Index(urlString[idx+4:], "/"); endIdx != -1 {
|
||||
urlString = urlString[:idx+4] + "REDACTED" + urlString[idx+4+endIdx:]
|
||||
} else if queryIdx := strings.Index(urlString[idx+4:], "?"); queryIdx != -1 {
|
||||
urlString = urlString[:idx+4] + "REDACTED" + urlString[idx+4+queryIdx:]
|
||||
} else {
|
||||
urlString = urlString[:idx+4] + "REDACTED"
|
||||
case "discord.com", "discordapp.com":
|
||||
// Discord webhook IDs and tokens follow /webhooks/ in both
|
||||
// unversioned and versioned API paths. Mask the entire suffix,
|
||||
// including escaped credentials and compatibility endpoint paths.
|
||||
if idx := strings.Index(parsed.Path, "/webhooks/"); idx != -1 {
|
||||
parsed.Path = parsed.Path[:idx] + "/webhooks/REDACTED"
|
||||
parsed.RawPath = ""
|
||||
urlString = parsed.String()
|
||||
}
|
||||
}
|
||||
|
||||
queryIndex := strings.Index(urlString, "?")
|
||||
if queryIndex == -1 {
|
||||
return urlString
|
||||
// Telegram also supports local API servers, so retain host-independent
|
||||
// masking, but inspect only the decoded path. Searching the whole URL
|
||||
// misses escaped prefixes and can mistake hostnames or query URLs for
|
||||
// bot credentials. Clear RawPath to prevent escaped secrets resurfacing.
|
||||
if idx := strings.Index(parsed.Path, "/bot"); idx != -1 {
|
||||
end := len(parsed.Path)
|
||||
if suffix := strings.Index(parsed.Path[idx+4:], "/"); suffix != -1 {
|
||||
end = idx + 4 + suffix
|
||||
}
|
||||
parsed.Path = parsed.Path[:idx+4] + "REDACTED" + parsed.Path[end:]
|
||||
parsed.RawPath = ""
|
||||
urlString = parsed.String()
|
||||
}
|
||||
|
||||
for _, parameter := range []string{"token", "apikey", "api_key", "key", "secret", "password"} {
|
||||
pattern := parameter + "="
|
||||
searchStart := queryIndex
|
||||
for {
|
||||
parameterIndex := strings.Index(urlString[searchStart:], pattern)
|
||||
if parameterIndex == -1 {
|
||||
break
|
||||
// Decode names exactly once, as net/url does, but retain the original
|
||||
// spelling, order and unrelated values in diagnostic URLs. Inspect every
|
||||
// occurrence rather than Query().Get(), which would miss repeated keys.
|
||||
parts := strings.Split(parsed.RawQuery, "&")
|
||||
changed := false
|
||||
for i, part := range parts {
|
||||
name, _, hasValue := strings.Cut(part, "=")
|
||||
decoded, err := url.QueryUnescape(name)
|
||||
if err != nil {
|
||||
return invalidWebhookURLDiagnostic
|
||||
}
|
||||
switch decoded {
|
||||
case "token", "apikey", "api_key", "key", "secret", "password":
|
||||
if hasValue {
|
||||
parts[i] = name + "=REDACTED"
|
||||
changed = true
|
||||
}
|
||||
parameterIndex += searchStart
|
||||
|
||||
if parameterIndex > 0 {
|
||||
previous := urlString[parameterIndex-1]
|
||||
if previous != '?' && previous != '&' {
|
||||
searchStart = parameterIndex + len(pattern)
|
||||
continue
|
||||
}
|
||||
}
|
||||
|
||||
valueStart := parameterIndex + len(pattern)
|
||||
valueEnd := valueStart
|
||||
for valueEnd < len(urlString) && urlString[valueEnd] != '&' && urlString[valueEnd] != '#' {
|
||||
valueEnd++
|
||||
}
|
||||
urlString = urlString[:valueStart] + "REDACTED" + urlString[valueEnd:]
|
||||
searchStart = valueStart + len("REDACTED")
|
||||
}
|
||||
}
|
||||
|
||||
if changed {
|
||||
parsed.RawQuery = strings.Join(parts, "&")
|
||||
return parsed.String()
|
||||
}
|
||||
return urlString
|
||||
}
|
||||
|
||||
|
||||
@@ -16,6 +16,15 @@ func TestRedactWebhookURLSecrets(t *testing.T) {
|
||||
input string
|
||||
want string
|
||||
}{
|
||||
"malformed query name": {input: "https://example.test/hook?%zz=secret", want: invalidWebhookURLDiagnostic},
|
||||
"encoded lookalike": {input: "https://example.test/hook?extra_%74oken=visible&channel=ops#fragment", want: "https://example.test/hook?extra_%74oken=visible&channel=ops#fragment"},
|
||||
"discord": {input: "https://discord.com/api/webhooks/123/discord-secret", want: "https://discord.com/api/webhooks/REDACTED"},
|
||||
"discord versioned": {input: "https://discord.com/api/v10/webhooks/123/discord-secret", want: "https://discord.com/api/v10/webhooks/REDACTED"},
|
||||
"discord legacy": {input: "https://discordapp.com/api/webhooks/123/discord-secret", want: "https://discordapp.com/api/webhooks/REDACTED"},
|
||||
"discord encoded": {input: "https://user:password@DISCORD.COM:443/api/webhooks/123/discord%2Dsecret", want: "https://REDACTED@DISCORD.COM:443/api/webhooks/REDACTED"},
|
||||
"discord query": {input: "https://discord.com/api/webhooks/123/discord-secret?wait=true&token=query-secret", want: "https://discord.com/api/webhooks/REDACTED?wait=true&token=REDACTED"},
|
||||
"discord lookalike": {input: "https://discord.com.example.org/api/webhooks/status", want: "https://discord.com.example.org/api/webhooks/status"},
|
||||
"discord unrelated": {input: "https://discord.com/api/status", want: "https://discord.com/api/status"},
|
||||
"slack": {input: "https://hooks.slack.com/services/T-test/B-test/slack-secret", want: "https://hooks.slack.com/services/REDACTED"},
|
||||
"gov slack": {input: "https://hooks.slack-gov.com/services/T-test/B-test/slack-secret?token=query-secret&channel=ops", want: "https://hooks.slack-gov.com/services/REDACTED?token=REDACTED&channel=ops"},
|
||||
"slack encoded path and authority": {input: "https://user:password@HOOKS.SLACK.COM:443/serv%69ces/T-test/B-test/slack%2Dsecret", want: "https://REDACTED@HOOKS.SLACK.COM:443/services/REDACTED"},
|
||||
@@ -46,6 +55,13 @@ func TestRedactWebhookURLSecrets(t *testing.T) {
|
||||
input: "https://gotify.example/message?token=gotify-secret",
|
||||
want: "https://gotify.example/message?token=REDACTED",
|
||||
},
|
||||
"telegram escaped prefix": {input: "https://api.telegram.org/%62ot123:telegram-secret/sendMessage", want: "https://api.telegram.org/botREDACTED/sendMessage"},
|
||||
"telegram escaped token": {input: "https://api.telegram.org/bot123:telegram%2Dsecret/sendMessage", want: "https://api.telegram.org/botREDACTED/sendMessage"},
|
||||
"telegram local escaped prefix": {input: "http://localhost:8081/%62ot123:telegram-secret/sendMessage", want: "http://localhost:8081/botREDACTED/sendMessage"},
|
||||
"telegram no method with URL query": {input: "https://api.telegram.org/bot123:telegram-secret?next=https://example.org/status", want: "https://api.telegram.org/botREDACTED?next=https://example.org/status"},
|
||||
"telegram fragment": {input: "https://api.telegram.org/bot123:telegram-secret#diagnostic", want: "https://api.telegram.org/botREDACTED#diagnostic"},
|
||||
"bot hostname is not a path": {input: "https://bot.example.org/hook?channel=ops", want: "https://bot.example.org/hook?channel=ops"},
|
||||
"bot query is not a path": {input: "https://example.org/hook?next=https://bot.example.org/status", want: "https://example.org/hook?next=https://bot.example.org/status"},
|
||||
"telegram path and query": {
|
||||
input: "https://api.telegram.org/bot123:secret/send?token=query-secret",
|
||||
want: "https://api.telegram.org/botREDACTED/send?token=REDACTED",
|
||||
@@ -180,3 +196,52 @@ func TestSlackWebhookDiagnosticsRedactPath(t *testing.T) {
|
||||
t.Fatalf("unsafe rate-limit diagnostic: %s", out)
|
||||
}
|
||||
}
|
||||
func TestDiscordWebhookDiagnosticsRedactPath(t *testing.T) {
|
||||
const webhookURL = "https://discord.com/api/webhooks/123/discord-secret"
|
||||
cause := errors.New("connection refused")
|
||||
original := &url.Error{Op: "Post", URL: webhookURL, Err: cause}
|
||||
redacted := redactWebhookTransportError(original)
|
||||
if strings.Contains(redacted.Error(), "discord-secret") || !strings.Contains(redacted.Error(), "/api/webhooks/REDACTED") {
|
||||
t.Fatalf("unsafe transport diagnostic: %v", redacted)
|
||||
}
|
||||
if original.URL != webhookURL || !errors.Is(redacted, cause) {
|
||||
t.Fatal("transport error identity or cause changed")
|
||||
}
|
||||
var captured bytes.Buffer
|
||||
logger := log.Logger
|
||||
log.Logger = zerolog.New(&captured)
|
||||
t.Cleanup(func() { log.Logger = logger })
|
||||
nm := &NotificationManager{webhookRateLimits: make(map[string]*webhookRateLimit)}
|
||||
for range WebhookRateLimitMax + 2 {
|
||||
nm.checkWebhookRateLimit(webhookURL)
|
||||
}
|
||||
out := captured.String()
|
||||
if !strings.Contains(out, "rate limit exceeded") || !strings.Contains(out, "/api/webhooks/REDACTED") || strings.Contains(out, "discord-secret") {
|
||||
t.Fatalf("unsafe rate-limit diagnostic: %s", out)
|
||||
}
|
||||
}
|
||||
|
||||
func TestTelegramWebhookDiagnosticsRedactPath(t *testing.T) {
|
||||
const webhookURL = "https://api.telegram.org/%62ot123:telegram-secret/sendMessage"
|
||||
cause := errors.New("connection refused")
|
||||
original := &url.Error{Op: "Post", URL: webhookURL, Err: cause}
|
||||
redacted := redactWebhookTransportError(original)
|
||||
if strings.Contains(redacted.Error(), "telegram-secret") || !strings.Contains(redacted.Error(), "/botREDACTED/sendMessage") {
|
||||
t.Fatalf("unsafe transport diagnostic: %v", redacted)
|
||||
}
|
||||
if original.URL != webhookURL || !errors.Is(redacted, cause) {
|
||||
t.Fatal("transport error identity or cause changed")
|
||||
}
|
||||
var captured bytes.Buffer
|
||||
logger := log.Logger
|
||||
log.Logger = zerolog.New(&captured)
|
||||
t.Cleanup(func() { log.Logger = logger })
|
||||
nm := &NotificationManager{webhookRateLimits: make(map[string]*webhookRateLimit)}
|
||||
for range WebhookRateLimitMax + 2 {
|
||||
nm.checkWebhookRateLimit(webhookURL)
|
||||
}
|
||||
out := captured.String()
|
||||
if !strings.Contains(out, "rate limit exceeded") || !strings.Contains(out, "/botREDACTED/sendMessage") || strings.Contains(out, "telegram-secret") {
|
||||
t.Fatalf("unsafe rate-limit diagnostic: %s", out)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3514,14 +3514,11 @@ func (s *Service) backfillAvailabilitySuggestions(ctx context.Context) {
|
||||
}
|
||||
|
||||
externalIP := s.getResourceExternalIP(req)
|
||||
suggestion := SuggestAvailabilityProbe(d, externalIP)
|
||||
if suggestion != nil {
|
||||
d.SuggestedAvailabilityProbe = suggestion
|
||||
if err := s.store.Save(d); err != nil {
|
||||
log.Warn().Err(err).Str("id", d.ID).Msg("Failed to save backfilled availability suggestion")
|
||||
} else {
|
||||
updated++
|
||||
}
|
||||
changed, err := s.store.backfillAvailabilitySuggestion(d.ID, externalIP)
|
||||
if err != nil {
|
||||
log.Warn().Err(err).Str("id", d.ID).Msg("Failed to save backfilled availability suggestion")
|
||||
} else if changed {
|
||||
updated++
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -8,6 +8,7 @@ import (
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"sync"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
@@ -2667,3 +2668,95 @@ func TestService_ListDiscoveriesByTarget_DoesNotBridgeSharedHostnames(t *testing
|
||||
t.Fatalf("expected estate B to keep finding its own record, got %d", len(own))
|
||||
}
|
||||
}
|
||||
|
||||
// Pause the first snapshot read, which backfill performs after Store.List.
|
||||
// Other reads remain available to the concurrent manual discovery.
|
||||
type backfillBarrierState struct {
|
||||
unifiedresources.ReadState
|
||||
first atomic.Bool
|
||||
listed chan struct{}
|
||||
resume chan struct{}
|
||||
}
|
||||
|
||||
func (s *backfillBarrierState) VMs() []*unifiedresources.VMView {
|
||||
if s.first.CompareAndSwap(false, true) {
|
||||
close(s.listed)
|
||||
<-s.resume
|
||||
}
|
||||
return s.ReadState.VMs()
|
||||
}
|
||||
|
||||
func TestService_BackfillPreservesConcurrentManualRepair(t *testing.T) {
|
||||
store, err := NewStore(t.TempDir())
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
service := NewService(store, nil, DefaultConfig())
|
||||
rs := readStateFromSnapshot(StateSnapshot{Containers: []Container{
|
||||
{VMID: 102, Name: "esphome", Node: "pve1", Status: "running"},
|
||||
}})
|
||||
// Set up the fixture before starting the asynchronous backfill.
|
||||
service.readState = rs
|
||||
service.SetCommandScanningEnabled(true)
|
||||
service.SetAIAnalyzer(&stubAnalyzer{response: `{}`})
|
||||
service.collectFingerprints(context.Background())
|
||||
id := MakeResourceID(ResourceTypeSystemContainer, "pve1", "102")
|
||||
fp, err := store.GetFingerprint(id)
|
||||
if err != nil || fp == nil {
|
||||
t.Fatalf("fingerprint: %v, %v", fp, err)
|
||||
}
|
||||
if err := store.Save(&ResourceDiscovery{
|
||||
ID: id, ResourceType: ResourceTypeSystemContainer, TargetID: "pve1", ResourceID: "102",
|
||||
Hostname: "esphome", ServiceType: "unknown", ServiceName: "Unknown Service", Category: CategoryUnknown,
|
||||
Fingerprint: fp.Hash, FingerprintedAt: fp.GeneratedAt, FingerprintSchemaVersion: fp.SchemaVersion,
|
||||
CLIAccessVersion: CLIAccessVersion,
|
||||
}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
barrier := &backfillBarrierState{ReadState: rs, listed: make(chan struct{}), resume: make(chan struct{})}
|
||||
var release sync.Once
|
||||
unblock := func() { release.Do(func() { close(barrier.resume) }) }
|
||||
service.SetReadState(barrier)
|
||||
t.Cleanup(func() { unblock(); service.backfillCancel(); <-service.backfillDone })
|
||||
select {
|
||||
case <-barrier.listed:
|
||||
case <-time.After(5 * time.Second):
|
||||
t.Fatal("backfill did not reach snapshot barrier")
|
||||
}
|
||||
summary, err := service.RunManualDiscoveryRefresh(context.Background())
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if summary.DiscoveredCount != 1 || summary.FailedCount != 0 {
|
||||
t.Fatalf("refresh: %+v", summary)
|
||||
}
|
||||
repaired, err := store.Get(id)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if repaired.ServiceType != "esphome" {
|
||||
t.Fatalf("manual repair failed: %q", repaired.ServiceType)
|
||||
}
|
||||
unblock()
|
||||
select {
|
||||
case <-service.backfillDone:
|
||||
case <-time.After(5 * time.Second):
|
||||
t.Fatal("backfill did not finish")
|
||||
}
|
||||
// Read through a new encrypted store as well as the live cache.
|
||||
restarted, err := NewStore(filepath.Dir(store.dataDir))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
for name, reader := range map[string]*Store{"cache": store, "disk": restarted} {
|
||||
got, err := reader.Get(id)
|
||||
if err != nil || got == nil {
|
||||
t.Fatalf("%s read: %v", name, err)
|
||||
}
|
||||
if got.ServiceType != repaired.ServiceType || got.ServiceName != repaired.ServiceName ||
|
||||
got.SuggestedURL != repaired.SuggestedURL || got.DiscoveryEngineVersion != repaired.DiscoveryEngineVersion {
|
||||
t.Errorf("%s: backfill overwrote manual repair: type=%q name=%q URL=%q engine=%d", name,
|
||||
got.ServiceType, got.ServiceName, got.SuggestedURL, got.DiscoveryEngineVersion)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -381,7 +381,11 @@ func (s *Store) marshalDiscoveryForStorage(discovery *ResourceDiscovery) ([]byte
|
||||
func (s *Store) Save(d *ResourceDiscovery) error {
|
||||
s.mu.Lock()
|
||||
defer s.mu.Unlock()
|
||||
return s.saveLocked(d)
|
||||
}
|
||||
|
||||
// saveLocked requires s.mu to be held for writing.
|
||||
func (s *Store) saveLocked(d *ResourceDiscovery) error {
|
||||
if d.ID == "" {
|
||||
return fmt.Errorf("discovery ID is required")
|
||||
}
|
||||
@@ -433,7 +437,11 @@ func (s *Store) Get(id string) (*ResourceDiscovery, error) {
|
||||
|
||||
s.mu.Lock()
|
||||
defer s.mu.Unlock()
|
||||
return s.getLocked(id)
|
||||
}
|
||||
|
||||
// getLocked requires s.mu to be held for writing (loads may migrate files).
|
||||
func (s *Store) getLocked(id string) (*ResourceDiscovery, error) {
|
||||
filePath := s.getFilePath(id)
|
||||
activePath := filePath
|
||||
data, migratedPlaintext, err := s.loadDiscoveryFileData(filePath, maxDiscoveryFileReadBytes)
|
||||
@@ -481,6 +489,30 @@ func (s *Store) Get(id string) (*ResourceDiscovery, error) {
|
||||
return cloneResourceDiscovery(&discovery), nil
|
||||
}
|
||||
|
||||
// backfillAvailabilitySuggestion reads, derives and persists under one lock.
|
||||
// A List snapshot is only a work list: never write its stale identity or resurrect
|
||||
// a deleted discovery. State/monitor reads must happen before taking this lock.
|
||||
func (s *Store) backfillAvailabilitySuggestion(id, externalIP string) (bool, error) {
|
||||
s.mu.Lock()
|
||||
defer s.mu.Unlock()
|
||||
current, err := s.getLocked(id)
|
||||
if err != nil || current == nil {
|
||||
return false, err
|
||||
}
|
||||
if current.SuggestedAvailabilityProbe != nil {
|
||||
return false, nil
|
||||
}
|
||||
suggestion := SuggestAvailabilityProbe(current, externalIP)
|
||||
if suggestion == nil {
|
||||
return false, nil
|
||||
}
|
||||
current.SuggestedAvailabilityProbe = suggestion
|
||||
if err := s.saveLocked(current); err != nil {
|
||||
return false, err
|
||||
}
|
||||
return true, nil
|
||||
}
|
||||
|
||||
// GetByResource retrieves a discovery by resource type and ID.
|
||||
func (s *Store) GetByResource(resourceType ResourceType, targetID, resourceID string) (*ResourceDiscovery, error) {
|
||||
id := MakeResourceID(resourceType, targetID, resourceID)
|
||||
|
||||
@@ -1234,3 +1234,95 @@ func TestStore_GetStaleResources(t *testing.T) {
|
||||
t.Fatalf("expected GetStaleResources to return list error")
|
||||
}
|
||||
}
|
||||
|
||||
func TestStore_BackfillAvailabilitySuggestionUsesCurrentRecord(t *testing.T) {
|
||||
for _, scenario := range []string{"new identity", "existing dismissed proposal", "deleted", "unsupported", "write failure"} {
|
||||
t.Run(scenario, func(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
store, err := NewStore(dir)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
id := MakeResourceID(ResourceTypeSystemContainer, "pve1", "102")
|
||||
old := &ResourceDiscovery{ID: id, Hostname: "esphome", ServiceType: "unknown"}
|
||||
if err := store.Save(old); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
work, err := store.List()
|
||||
if err != nil || len(work) != 1 {
|
||||
t.Fatalf("List: %v, %v", work, err)
|
||||
}
|
||||
current := cloneResourceDiscovery(old)
|
||||
current.ServiceType = "redis"
|
||||
current.ServiceName = "New Redis"
|
||||
current.UserNotes = "operator note after list"
|
||||
if scenario == "existing dismissed proposal" {
|
||||
current.SuggestedAvailabilityProbe = SuggestAvailabilityProbe(current, "192.0.2.20")
|
||||
current.DismissedAvailabilityProbeFingerprint = current.SuggestedAvailabilityProbe.EvidenceFingerprint
|
||||
}
|
||||
if scenario == "unsupported" {
|
||||
current.ServiceType = "unknown"
|
||||
current.Hostname = "unidentified"
|
||||
}
|
||||
if err := store.Save(current); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
before, err := store.Get(id)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if scenario == "deleted" {
|
||||
if err := store.Delete(id); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
if scenario == "write failure" {
|
||||
// A directory at the temporary file path causes a real persistence error.
|
||||
if err := os.Mkdir(store.getFilePath(id)+".tmp", 0700); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
changed, err := store.backfillAvailabilitySuggestion(work[0].ID, "192.0.2.10")
|
||||
if scenario == "write failure" {
|
||||
if err == nil || changed {
|
||||
t.Fatalf("expected failed write, got changed=%v err=%v", changed, err)
|
||||
}
|
||||
} else if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if changed != (scenario == "new identity") {
|
||||
t.Fatalf("unexpected changed=%v", changed)
|
||||
}
|
||||
restarted, err := NewStore(dir)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
for name, reader := range map[string]*Store{"cache": store, "disk": restarted} {
|
||||
got, err := reader.Get(id)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if scenario == "deleted" {
|
||||
if got != nil {
|
||||
t.Errorf("%s: deleted discovery resurrected", name)
|
||||
}
|
||||
continue
|
||||
}
|
||||
want := cloneResourceDiscovery(before)
|
||||
if scenario == "new identity" {
|
||||
want.SuggestedAvailabilityProbe = SuggestAvailabilityProbe(before, "192.0.2.10")
|
||||
want.UpdatedAt = got.UpdatedAt
|
||||
if got.SuggestedAvailabilityProbe == nil || got.SuggestedAvailabilityProbe.Port != 6379 {
|
||||
t.Fatalf("%s: suggestion not derived from current Redis identity", name)
|
||||
}
|
||||
}
|
||||
// JSON comparison ignores time.Time's process-local monotonic clock.
|
||||
gotJSON, _ := json.Marshal(got)
|
||||
wantJSON, _ := json.Marshal(want)
|
||||
if string(gotJSON) != string(wantJSON) {
|
||||
t.Errorf("%s: backfill changed unrelated/current fields", name)
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -310,6 +310,14 @@ class ReleasePromotionPolicyTest(unittest.TestCase):
|
||||
):
|
||||
self.skipTest("staged governance inputs missing; see test_staged_governance_inputs_are_present")
|
||||
|
||||
def test_release_workflow_supports_reviewed_immutable_snapshots(self) -> None:
|
||||
from release_snapshot import check_workflow
|
||||
|
||||
with tempfile.TemporaryDirectory() as directory:
|
||||
workflow = Path(directory) / "create-release.yml"
|
||||
workflow.write_text(read(".github/workflows/create-release.yml"))
|
||||
check_workflow(workflow)
|
||||
|
||||
def test_staged_governance_inputs_are_present(self) -> None:
|
||||
if STAGED_GOVERNANCE_INPUT_ERRORS:
|
||||
self.fail(
|
||||
|
||||
@@ -0,0 +1,100 @@
|
||||
#!/usr/bin/env python3
|
||||
"""Bind release execution to a reviewed snapshot, independently of a moving train."""
|
||||
from __future__ import annotations
|
||||
|
||||
import argparse
|
||||
import json
|
||||
import os
|
||||
from pathlib import Path
|
||||
import re
|
||||
import subprocess
|
||||
|
||||
|
||||
SHA = re.compile(r"[0-9a-f]{40}")
|
||||
SNAPSHOT_REF = re.compile(r"release-candidate/[0-9A-Za-z._-]+")
|
||||
SOURCE_BRANCH = re.compile(r"main|release/v[0-9]+\.[0-9]+")
|
||||
|
||||
|
||||
def source_branch(ref: str, requested: str, pull_request: str) -> str:
|
||||
if not ref.startswith("refs/heads/"):
|
||||
raise ValueError("release dispatch must name a branch ref")
|
||||
branch = ref.removeprefix("refs/heads/")
|
||||
if SNAPSHOT_REF.fullmatch(branch):
|
||||
if not SOURCE_BRANCH.fullmatch(requested) or not re.fullmatch(r"[1-9][0-9]*", pull_request):
|
||||
raise ValueError("snapshot dispatch requires its governed source branch and merged pull request")
|
||||
return requested
|
||||
if requested or pull_request:
|
||||
raise ValueError("snapshot provenance inputs require a reserved release-candidate ref")
|
||||
if not SOURCE_BRANCH.fullmatch(branch):
|
||||
raise ValueError("release dispatch is outside a governed source branch")
|
||||
return branch
|
||||
|
||||
|
||||
def reviewed_merge(pr: dict, *, repository: str, ref: str, branch: str, sha: str) -> str:
|
||||
if not SHA.fullmatch(sha):
|
||||
raise ValueError("source must be an exact commit")
|
||||
head, base = pr.get("head", {}), pr.get("base", {})
|
||||
if pr.get("merged") is not True or pr.get("state") != "closed":
|
||||
raise ValueError("release snapshot pull request is not merged")
|
||||
if head.get("sha") != sha or head.get("ref") != ref.removeprefix("refs/heads/"):
|
||||
raise ValueError("pull request does not identify the dispatched snapshot")
|
||||
if base.get("ref") != branch:
|
||||
raise ValueError("pull request belongs to another release line")
|
||||
if any(part.get("repo", {}).get("full_name") != repository for part in (head, base)):
|
||||
raise ValueError("release snapshot must come from the canonical repository")
|
||||
merge = pr.get("merge_commit_sha", "")
|
||||
if not isinstance(merge, str) or not SHA.fullmatch(merge):
|
||||
raise ValueError("pull request has no exact merge commit")
|
||||
return merge
|
||||
|
||||
|
||||
def check_workflow(path: Path) -> None:
|
||||
# BaseLoader preserves the YAML `on` key rather than treating it as a bool.
|
||||
import yaml
|
||||
|
||||
workflow = yaml.load(path.read_text(), Loader=yaml.BaseLoader)
|
||||
inputs = workflow["on"]["workflow_dispatch"]["inputs"]
|
||||
for name in ("expected_source_sha", "release_source_branch", "release_pull_request"):
|
||||
if inputs.get(name, {}).get("type") != "string":
|
||||
raise ValueError(f"release workflow lacks snapshot input {name}")
|
||||
steps = workflow["jobs"]["prepare"]["steps"]
|
||||
guard = next((step for step in steps if step.get("id") == "snapshot"), {})
|
||||
if "python3 scripts/release_control/release_snapshot.py" not in guard.get("run", ""):
|
||||
raise ValueError("release workflow does not execute the snapshot provenance guard")
|
||||
expected = {"RELEASE_SOURCE_BRANCH": "${{ inputs.release_source_branch }}", "RELEASE_PULL_REQUEST": "${{ inputs.release_pull_request }}"}
|
||||
if any(guard.get("env", {}).get(key) != value for key, value in expected.items()):
|
||||
raise ValueError("release workflow does not bind snapshot provenance inputs")
|
||||
|
||||
|
||||
def main() -> None:
|
||||
parser = argparse.ArgumentParser(description=__doc__)
|
||||
parser.add_argument("--check-workflow", type=Path)
|
||||
args = parser.parse_args()
|
||||
if args.check_workflow:
|
||||
check_workflow(args.check_workflow)
|
||||
return
|
||||
|
||||
ref = os.environ["GITHUB_REF"]
|
||||
requested = os.environ.get("RELEASE_SOURCE_BRANCH", "")
|
||||
pr_number = os.environ.get("RELEASE_PULL_REQUEST", "")
|
||||
branch = source_branch(ref, requested, pr_number)
|
||||
if requested:
|
||||
repository = os.environ["GITHUB_REPOSITORY"]
|
||||
if repository != "rcourtman/Pulse":
|
||||
raise ValueError("snapshot releases are restricted to the canonical Pulse repository")
|
||||
sha = os.environ["GITHUB_SHA"]
|
||||
if os.environ["GITHUB_WORKFLOW_SHA"] != sha:
|
||||
raise ValueError("workflow and source must be the same immutable snapshot")
|
||||
pr = json.loads(subprocess.check_output(["gh", "api", f"repos/{repository}/pulls/{pr_number}"], text=True))
|
||||
merge = reviewed_merge(pr, repository=repository, ref=ref, branch=branch, sha=sha)
|
||||
subprocess.run(["git", "fetch", "--quiet", "--no-tags", f"https://github.com/{repository}.git", f"refs/heads/{branch}"], check=True)
|
||||
# Later merges are allowed. A rewrite that removes the reviewed merge is not.
|
||||
subprocess.run(["git", "merge-base", "--is-ancestor", sha, merge], check=True)
|
||||
subprocess.run(["git", "merge-base", "--is-ancestor", merge, "FETCH_HEAD"], check=True)
|
||||
with Path(os.environ["GITHUB_OUTPUT"]).open("a") as output:
|
||||
output.write(f"source_branch={branch}\n")
|
||||
print(f"Release source is bound to {os.environ['GITHUB_SHA']} from {branch}")
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
main()
|
||||
@@ -0,0 +1,85 @@
|
||||
"""Release provenance stays bound to the reviewed commit as its train advances."""
|
||||
from copy import deepcopy
|
||||
import json
|
||||
import os
|
||||
from pathlib import Path
|
||||
import subprocess
|
||||
import tempfile
|
||||
import unittest
|
||||
from unittest.mock import patch
|
||||
|
||||
import release_snapshot as snapshot
|
||||
|
||||
|
||||
class SnapshotIdentityTest(unittest.TestCase):
|
||||
def setUp(self):
|
||||
self.sha = 'a' * 40
|
||||
self.merge = 'b' * 40
|
||||
self.ref = 'refs/heads/release-candidate/packet-1'
|
||||
self.pr = {
|
||||
'state': 'closed', 'merged': True, 'merge_commit_sha': self.merge,
|
||||
'head': {'sha': self.sha, 'ref': 'release-candidate/packet-1', 'repo': {'full_name': 'rcourtman/Pulse'}},
|
||||
'base': {'ref': 'release/v6.4', 'repo': {'full_name': 'rcourtman/Pulse'}},
|
||||
}
|
||||
|
||||
def verify(self, pr):
|
||||
return snapshot.reviewed_merge(pr, repository='rcourtman/Pulse', ref=self.ref,
|
||||
branch='release/v6.4', sha=self.sha)
|
||||
|
||||
def test_merged_snapshot_identity(self):
|
||||
self.assertEqual(self.merge, self.verify(self.pr))
|
||||
self.assertEqual('release/v6.4', snapshot.source_branch(self.ref, 'release/v6.4', '42'))
|
||||
self.assertEqual('main', snapshot.source_branch('refs/heads/main', '', ''))
|
||||
|
||||
def test_unreviewed_or_retargeted_identity_is_rejected(self):
|
||||
changes = [('merged', False), ('state', 'open'), ('merge_commit_sha', ''),
|
||||
('head.sha', 'c' * 40), ('head.ref', 'release-candidate/other'),
|
||||
('base.ref', 'main'), ('head.repo.full_name', 'someone/Pulse'),
|
||||
('base.repo.full_name', 'someone/Pulse')]
|
||||
for path, value in changes:
|
||||
with self.subTest(path=path):
|
||||
pr = deepcopy(self.pr)
|
||||
target = pr
|
||||
fields = path.split('.')
|
||||
for field in fields[:-1]:
|
||||
target = target[field]
|
||||
target[fields[-1]] = value
|
||||
with self.assertRaises(ValueError):
|
||||
self.verify(pr)
|
||||
|
||||
def test_unbound_dispatch_inputs_are_rejected(self):
|
||||
for args in [(self.ref, '', '42'), (self.ref, 'main', ''),
|
||||
(self.ref, 'feature/other', '42'), (self.ref, 'main', '../42'),
|
||||
('refs/heads/main', 'main', '42'), ('refs/tags/v6.4.4-beta.1', '', '')]:
|
||||
with self.subTest(args=args), self.assertRaises(ValueError):
|
||||
snapshot.source_branch(*args)
|
||||
|
||||
def test_source_workflow_supports_snapshot_inputs_before_qualification(self):
|
||||
path = Path(__file__).resolve().parents[2] / '.github/workflows/create-release.yml'
|
||||
snapshot.check_workflow(path)
|
||||
with tempfile.TemporaryDirectory() as raw:
|
||||
old = Path(raw) / 'workflow.yml'
|
||||
old.write_text(path.read_text().replace('release_pull_request:', 'removed_input:', 1))
|
||||
with self.assertRaises(ValueError):
|
||||
snapshot.check_workflow(old)
|
||||
|
||||
def test_workflow_verifies_ancestry_without_requiring_current_tip(self):
|
||||
with tempfile.TemporaryDirectory() as raw:
|
||||
output = Path(raw) / 'output'
|
||||
env = {'GITHUB_REF': self.ref, 'RELEASE_SOURCE_BRANCH': 'release/v6.4',
|
||||
'RELEASE_PULL_REQUEST': '42', 'GITHUB_REPOSITORY': 'rcourtman/Pulse',
|
||||
'GITHUB_SHA': self.sha, 'GITHUB_WORKFLOW_SHA': self.sha,
|
||||
'GITHUB_OUTPUT': str(output)}
|
||||
with patch.dict(os.environ, env), patch('sys.argv', ['release_snapshot.py']), \
|
||||
patch.object(subprocess, 'check_output', return_value=json.dumps(self.pr)), \
|
||||
patch.object(subprocess, 'run') as run:
|
||||
snapshot.main()
|
||||
self.assertEqual('source_branch=release/v6.4\n', output.read_text())
|
||||
commands = [call.args[0] for call in run.call_args_list]
|
||||
self.assertIn(['git', 'merge-base', '--is-ancestor', self.sha, self.merge], commands)
|
||||
self.assertIn(['git', 'merge-base', '--is-ancestor', self.merge, 'FETCH_HEAD'], commands)
|
||||
self.assertFalse(any('reset' in command or 'checkout' in command for command in commands))
|
||||
|
||||
|
||||
if __name__ == '__main__':
|
||||
unittest.main()
|
||||
Reference in New Issue
Block a user