Skip to content

Commit 214220c

Browse files
fix: an unreadable own label refuses the launch instead of falling through
`find_release` gave the provider's own `<provider>/…` token the last word, but only when that token parsed. When it did not, the loop fell through and kept scanning, so any other release in the banner could stand in for the one the provider had just named. `omp/18.1.0-rc1 18.0.9` admitted on the stray `18.0.9` and launched a provider that reported itself as an unverified 18.1 pre-release — the exact outcome the function's own doc comment promises to prevent. The label now decides outright: if what the provider named cannot be parsed, the answer is `None` and the caller fails closed. Both shipped banners are unaffected, which was checked rather than assumed — omp prints `omp/18.0.11`, which parses, and opencode prints a bare `1.18.25`, which never enters the labelled branch at all. Latent today because the real binaries print one token each, but DQ-OMP-5 is open on precisely the update banner that would add a second one, and the exact-version gate that used to mask this is gone: keying on the minor means a stray token only has to land in an admitted series, not match a version exactly. Found by independent verification of #370 as mutation S6: deleting the fall-through left the suite byte-identical, so nothing asserted it. Two tests now do, one on the parser and one driving the production gate through a fake provider. Both go red under S6 in either shape — restoring the `continue`, or deleting the labelled branch outright. agent-identity: dev3.dotfiles.omp.admission-18-0-9.worker agent-persona: worker agent-supervisor: dev3.dotfiles-lead agent-tool: Claude Code agent-tool-version: 2.1.251 agent-runtime: Claude Code 2.1.251 tooling-profile: dotfiles@6048b77
1 parent c4d6f7a commit 214220c

3 files changed

Lines changed: 59 additions & 9 deletions

File tree

docs/vrs/06-omp-driver/spec.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,12 @@ Shape of `pi_session.rs`:
5353
from decaying into "starts with 18" — minors are compared numerically (`18.10` is not `18.1`),
5454
exactly three components are required, and a pre-release or build-metadata suffix
5555
(`18.0.9-rc1`) does not parse at all, so it is never admitted as its base release.
56+
Which token the release is read FROM matters as much as how it parses: an `omp/<release>`
57+
token is omp naming itself and decides outright, and if what it named cannot be parsed the
58+
gate refuses rather than reading some other token in the banner. Otherwise `omp/18.1.0-rc1
59+
18.0.9` would launch an unverified provider on the strength of a version omp never claimed —
60+
which is the shape DQ-OMP-5's update banner could produce. With no own label, every parseable
61+
release in the banner must agree.
5662
- Injects the channel extension from the verified hook set (`with_channel_extension` shape —
5763
resolved from this binary's immutable asset, never a catalog-pinned path).
5864
- Applies offline defaults (`PI_OFFLINE=1`, `PI_SKIP_VERSION_CHECK=1`) unless the operator's

src/harness_version.rs

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -68,22 +68,25 @@ pub fn parse_release(token: &str) -> Option<Release> {
6868
/// unverified 18.1. So the provider's own label wins, and an unlabelled banner is only trusted
6969
/// when it is unambiguous:
7070
///
71-
/// 1. a `<provider>/<release>` token is authoritative — it is the provider naming itself;
72-
/// 2. otherwise every parseable release must agree, since a single release repeated across lines
73-
/// is still unambiguous;
74-
/// 3. anything else (no release, or two disagreeing ones) is `None`, and the caller fails closed.
71+
/// 1. a `<provider>/…` token DECIDES, because it is the provider naming itself. If what it named
72+
/// parses, that is the answer; if it does not, the answer is `None` — the search stops rather
73+
/// than falling through, or `omp/18.1.0-rc1 18.0.9` would admit on a release omp never
74+
/// claimed;
75+
/// 2. with no own label, every parseable release must agree, since a single release repeated
76+
/// across lines is still unambiguous;
77+
/// 3. anything else (no release, an unreadable own label, or two disagreeing ones) is `None`, and
78+
/// the caller fails closed.
7579
pub fn find_release<'a>(printed: &'a str, provider: &str) -> Option<(&'a str, Release)> {
7680
let labelled = format!("{provider}/");
7781
let mut unlabelled: Option<(&'a str, Release)> = None;
7882
let mut ambiguous = false;
7983

8084
for token in printed.split_whitespace() {
8185
if let Some(rest) = token.strip_prefix(labelled.as_str()) {
82-
// The provider naming itself outranks anything else in the banner.
83-
if let Some(release) = parse_release(rest) {
84-
return Some((rest, release));
85-
}
86-
continue;
86+
// The provider naming itself outranks anything else in the banner — including the
87+
// case where what it named cannot be read. Falling through to the rest of the banner
88+
// would let a stray token stand in for a release the provider never claimed.
89+
return parse_release(rest).map(|release| (rest, release));
8790
}
8891
let bare = token.strip_prefix('v').unwrap_or(token);
8992
let Some(release) = parse_release(bare) else {
@@ -193,6 +196,29 @@ mod tests {
193196
);
194197
}
195198

199+
/// The provider naming itself with something this parser cannot read is a REFUSAL, not a
200+
/// reason to keep looking. Falling through to the rest of the banner is how the gate ends up
201+
/// bound to a token the provider never claimed: `omp/18.1.0-rc1 18.0.9` would admit on the
202+
/// stray `18.0.9` and launch a provider that just said it was `18.1.0-rc1`. DQ-OMP-5 leaves
203+
/// open whether omp's update banner adds exactly such a second token.
204+
#[test]
205+
fn an_unreadable_own_label_fails_closed_instead_of_falling_through() {
206+
// The banner from the finding: an own label that does not parse, plus a stray release
207+
// whose minor IS admitted. Reading the stray one is the whole bug.
208+
assert!(find_release("omp/18.1.0-rc1 18.0.9", "omp").is_none());
209+
// Not specific to pre-releases: any unreadable own label ends the search.
210+
assert!(find_release("omp/nightly 18.0.9", "omp").is_none());
211+
assert!(find_release("omp/ 18.0.9", "omp").is_none());
212+
// The same parser backs the opencode gate, so it fails closed the same way.
213+
assert!(find_release("opencode/1.18.25-beta 1.18.19", "opencode").is_none());
214+
// An own label that DOES parse still decides, so the refusal above is about
215+
// unreadability and not about labels in general.
216+
assert_eq!(
217+
find_release("omp/18.0.11 18.0.9", "omp").unwrap().0,
218+
"18.0.11"
219+
);
220+
}
221+
196222
/// A label that belongs to a DIFFERENT provider must not be read as this provider's version.
197223
#[test]
198224
fn another_providers_label_does_not_satisfy_this_provider() {

src/omp_session.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,24 @@ mod tests {
388388
);
389389
}
390390

391+
/// The refusal must survive omp naming itself with something unreadable. Found by independent
392+
/// verification of #370 (mutation S6): deleting the labelled fall-through left the suite
393+
/// byte-identical, and driving this gate with `omp/18.1.0-rc1 18.0.9` ADMITTED the launch,
394+
/// bound to the stray token rather than to the release omp reported for itself. Latent while
395+
/// the shipped binary prints one token, but DQ-OMP-5 is open on precisely the update banner
396+
/// that would add a second.
397+
#[test]
398+
fn an_unreadable_own_label_cannot_be_rescued_by_a_stray_admitted_version() {
399+
let fake = FakeExecutable::new("#!/bin/sh\nprintf 'omp/18.1.0-rc1 18.0.9\\n'\n");
400+
let error = verify_supported_version(fake.path().to_str().unwrap())
401+
.expect_err("an unreadable own label must not admit on a stray 18.0.9")
402+
.to_string();
403+
assert!(
404+
error.contains("no unambiguous omp release"),
405+
"the refusal must say the banner named no release it could read: {error}"
406+
);
407+
}
408+
391409
/// Minors are compared as numbers. `18.10` must not pass on the strength of admitted `18.0`,
392410
/// which is how a minor gate would decay into "accept anything that starts with 18".
393411
#[test]

0 commit comments

Comments
 (0)