#1103 Version management hardening: stricter API input, pin retry rule, audit and test fixes Pull request opened by javuto Version management hardening: stricter API input, pin retry rule, audit and test fixes Follow-up to #1102. Small hardening items found in the final reviews of the osquery version management change. No new endpoints and no wire contract changes. Problem • A pin's expiry change retried a rejected target: every

SetPin

call generated a new

request_id

, so extending a pin's expiry silently made osctrld retry a version the host had already rejected. • Loose API input: an unknown field in a PUT body was ignored, so a misspelled

expires_at

created a pin that never expires. Duplicate library entries resolved silently (last wins), the

{version}

path parameter reached the database unvalidated, and an environment target PUT silently dropped

expires_at

. • Misleading audit rows: deleting an unset environment target or an absent pin wrote an audit row claiming a change. • Fragile in-use guard: the check that blocks changing or deleting a version in use compared pin expiry in SQL, which is a text comparison on SQLite across zone offsets. • Smaller gaps: pin denials had no audit assertion, one test used a 50 ms sleep, and the

no_digest_for_target

text did not name the version to add. What changes Pin request id rule (

pkg/osqueryversions

) •

SetPin

keeps the stored

request_id

only when the stored pin is in the same environment, has the same version, and the new expiry differs from the stored one (set vs unset counts as different). The row is updated in place. • A first pin, a different version, an identical re-save (same version and same expiry), or a pin row left over from another environment (pins are keyed by node UUID) gets a new id. An identical re-save is how an operator deliberately retries a rejected target. • Expiries less than a second apart count as the same, because MySQL and Postgres round fractional seconds. • Documented in the swag description of the node pin PUT,

ARCHITECTURE.md

and a hint under the pin control in the UI. API hygiene (

cmd/api/handlers/osquery_versions.go

) • Request bodies use

DisallowUnknownFields()

and reject trailing data with 400. Oversized padding after a valid object still gets 413, and a trailing newline is fine. • The

{version}

path parameter is validated with

ValidVersion

before use (400 instead of a 404 from the database). • Library PUT rejects duplicate

(platform, arch, format)

keys, enforced in

PutEntries

. Since there are exactly five supported combinations, this also caps a request at five entries, and the audit line now lists exactly what was written. • The environment target PUT answers 400 when

expires_at

is present (environment targets do not expire; use a node pin). •

ClearEnvTarget

and

DeletePin

report whether a row was removed. The handlers still answer 200 but audit only when something was removed. • Database errors answer 500. This already held in practice, and the call sites now say so explicitly, with a regression test that fails each table in turn. • Pin denials write a

Denied

audit row for GET, PUT and DELETE (now asserted). Smaller fixes • The in-use guard reads the pins for a version and filters expiry with

Expired(now)

in Go. A null expiry never expires. •

TestDeleteVersionExpiredPinNotInUse

inserts an already-expired pin directly instead of sleeping. •

no_digest_for_target

now reads "Add the SHA-256 for osquery ‹target version› (for this host's platform and architecture) to the library." It stays in the single reason-wording map and renders as plain text. • Added tests for gaps in the earlier review: • another environment's pin falls back to the environment target; • a pin expiring exactly at now counts as expired; • an unexpired pin blocks a digest change; • two missing

ValidUpgrade

rows; • the 413 status is pinned; • the never-404 sweep also asserts not 405; • an unsupported platform from an unknown node is 400. Security and operational impact • Stricter API: clients that sent extra fields, duplicate library entries, or

expires_at

on an environment target now get 400, and deleting an unset target or pin no longer writes an audit row. The frontend sends none of these. The bodies match the request structs, and the swag descriptions say so. • No permission changes: authorization, scoping and audit content are unchanged apart from the rows above. The

request_id

rule only affects whether osctrld retries a rejected target. Known limits (deferred) • The pin form in the UI does not prefill the expiry, so retrying a pinned version through the UI means retyping the exact expiry minute. A pin created through the API with seconds in its expiry cannot be matched from the UI. Remove and re-pin is the other way. • The library page allows the same package in two rows. It now fails at save with a server 400 instead of silently keeping the last one. •

ListVersions

loads pin rows per version to compute

in_use

. This is fine at current scale. • Two simultaneous library writes can show a slightly wrong "was" digest in the audit line. Validation • `go test ./... -count=1`: 58 packages pass, 0 failures (also with

-race

on

pkg/osqueryversions

and

cmd/api/handlers

) •

go vet ./...

,

gofmt -l pkg cmd

(empty), `golangci-lint run ./...`: 0 issues • `make openapi-check`: clean. The diff is limited to descriptions and the new 400 on library DELETE. • Frontend: 602 unit tests pass and

tsc

is clean • New tests, each seen failing first unless noted: • Pin rule: a first pin gets an id; an expiry added, removed or changed keeps it; an identical re-save (with or without expiry) and a different version change it; a pin left from another environment gets a new id and moves to the new environment; tolerance cases (.4 s across a second boundary, .9995 and .9999 versus 1 s rounding, 1.5 s apart). • Request bodies: unknown fields and trailing data rejected on all three PUTs, with nothing audited and no expiry set; oversized padding gives 413 and a trailing newline gives 200. • Input checks: invalid version paths give 400; too many or duplicate entries give 400 with nothing stored or audited; an environment target with

expires_at

gives 400 with nothing stored or audited. • Deletes and errors: deleting nothing is not audited; each table failing in turn gives 500 (this one passed immediately, as regression cover). • Guards: a pin expiry stored in a different zone offset still blocks changes to its version, and pin denials are audited. • UI: the reason text names the target version, and the pin hint is shown. jmpsec/osctrl