Skip to content

Commit 4e146dd

Browse files
Fix Go vex of +incompatible module patches (#484) (#1427)
* Start fix for #484 Assisted-by: Claude Code:claude-opus-5-5 * Fix vex of +incompatible Go module patches The patch API keys a +incompatible Go module as @v2.0.0%2Bincompatible, but go.mod spells the version with a literal +. vex compared the two spellings as plain strings, so an applied agent-mode Go patch for such a module was never verified: vex said not_applied and exited 1, leaving the patch out of the OpenVEX doc. Match Go purls by release identity (PurlKey) when looking up the manifest record for a go-patches replace, and in the go.work override filter, so a go.work replace that overrides a %2B-keyed patch is still caught and never attested. Fixes #484 Assisted-by: Claude Code:claude-opus-5-5 * Report Go vex omissions under the record's purl Follow-up to the +incompatible fix. When go.work overrides a Go patch, or the build selects another version, vex now names the omitted patch by the manifest's own purl (with %2B), not the go.mod spelling, so --json consumers can join omissions to manifest keys. The copy-dir lookup now maps every record spelling of the release, deterministically. Assisted-by: Claude Code:claude-opus-5-5 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent cc841a1 commit 4e146dd

2 files changed

Lines changed: 173 additions & 43 deletions

File tree

‎crates/socket-patch-cli/src/commands/vex.rs‎

Lines changed: 70 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -818,7 +818,8 @@ async fn generate_vex(
818818
// conflict with another member's (#393, #458, #531) is not what the
819819
// build links, however intact its copy or pin: never attest it.
820820
if !common.is_global() {
821-
use socket_patch_core::utils::purl::{build_golang_purl, strip_purl_qualifiers};
821+
use socket_patch_core::utils::purl::build_golang_purl;
822+
use socket_patch_core::utils::purl_key::PurlKey;
822823
for issue in socket_patch_core::vendor::go_consumer_sync::workspace_audit(&common.cwd).await
823824
{
824825
let socket_patch_core::vendor::go_consumer_sync::GoSyncIssue::Workspace {
@@ -829,19 +830,22 @@ async fn generate_vex(
829830
else {
830831
continue;
831832
};
832-
let purl = build_golang_purl(module, version);
833-
let before = outcome.applied.len();
834-
outcome
835-
.applied
836-
.retain(|p| strip_purl_qualifiers(p) != purl.as_str());
837-
if outcome.applied.len() == before {
838-
continue;
833+
// PurlKey, not string equality: go.mod spells `+incompatible`
834+
// with a literal `+`, the manifest key with `%2B` (#484). Report
835+
// each omission under the record's own spelling.
836+
let wanted = PurlKey::new(&build_golang_purl(module, version));
837+
let (overridden, kept): (Vec<String>, Vec<String>) =
838+
std::mem::take(&mut outcome.applied)
839+
.into_iter()
840+
.partition(|p| PurlKey::new(p) == wanted);
841+
outcome.applied = kept;
842+
for purl in overridden {
843+
note_warning(warnings, common, issue.code(), format!("{purl}: {issue}"));
844+
outcome.failed.push(FailedPatch {
845+
purl,
846+
reason: GO_WORKSPACE_OVERRIDE.to_string(),
847+
});
839848
}
840-
note_warning(warnings, common, issue.code(), format!("{purl}: {issue}"));
841-
outcome.failed.push(FailedPatch {
842-
purl,
843-
reason: GO_WORKSPACE_OVERRIDE.to_string(),
844-
});
845849
}
846850
}
847851

@@ -859,30 +863,38 @@ async fn generate_vex(
859863
let Some(version) = entry.version.as_deref().filter(|_| entry.socket_owned()) else {
860864
continue;
861865
};
862-
let purl = build_golang_purl(&entry.module, version);
863-
if !outcome.applied.iter().any(|p| PurlKey::same(p, &purl)) {
866+
let wanted = PurlKey::new(&build_golang_purl(&entry.module, version));
867+
if !outcome.applied.iter().any(|p| PurlKey::new(p) == wanted) {
864868
continue;
865869
}
866870
let Some(Selection::Other(selected)) =
867871
project_selection(&common.cwd, &entry.module, version).await
868872
else {
869873
continue;
870874
};
871-
outcome.applied.retain(|p| !PurlKey::same(p, &purl));
872-
note_warning(
873-
warnings,
874-
common,
875-
GO_VERSION_NOT_SELECTED,
876-
format!(
877-
"{purl}: the build selects {} {selected}, so the replace pinned to \
878-
{version} is ignored and the build links the unpatched module",
879-
entry.module
880-
),
881-
);
882-
outcome.failed.push(FailedPatch {
883-
purl,
884-
reason: GO_VERSION_NOT_SELECTED.to_string(),
885-
});
875+
// Report each omission under the record's own spelling (`%2B`
876+
// for `+incompatible`, #484), not go.mod's.
877+
let (unselected, kept): (Vec<String>, Vec<String>) =
878+
std::mem::take(&mut outcome.applied)
879+
.into_iter()
880+
.partition(|p| PurlKey::new(p) == wanted);
881+
outcome.applied = kept;
882+
for purl in unselected {
883+
note_warning(
884+
warnings,
885+
common,
886+
GO_VERSION_NOT_SELECTED,
887+
format!(
888+
"{purl}: the build selects {} {selected}, so the replace pinned to \
889+
{version} is ignored and the build links the unpatched module",
890+
entry.module
891+
),
892+
);
893+
outcome.failed.push(FailedPatch {
894+
purl,
895+
reason: GO_VERSION_NOT_SELECTED.to_string(),
896+
});
897+
}
886898
}
887899
}
888900

@@ -1659,10 +1671,19 @@ async fn synthesize_go_patches(
16591671
are_safe_redirect_coords, copy_dir_for,
16601672
};
16611673
use socket_patch_core::utils::purl::build_golang_purl;
1674+
use socket_patch_core::utils::purl_key::PurlKey;
16621675
use socket_patch_core::vendor::go_mod_edit::{
16631676
read_replace_entries, ReplaceOwner, GO_PATCHES_DIR,
16641677
};
16651678

1679+
let mut by_release: HashMap<PurlKey, Vec<&String>> = HashMap::new();
1680+
for key in manifest
1681+
.patches
1682+
.keys()
1683+
.filter(|k| k.starts_with("pkg:golang/"))
1684+
{
1685+
by_release.entry(PurlKey::new(key)).or_default().push(key);
1686+
}
16661687
let mut go_patches = HashMap::new();
16671688
for entry in read_replace_entries(&common.cwd).await {
16681689
if entry.owner != Some(ReplaceOwner::GoPatches) {
@@ -1671,26 +1692,32 @@ async fn synthesize_go_patches(
16711692
let Some(version) = entry.version.as_deref() else {
16721693
continue;
16731694
};
1674-
let purl = build_golang_purl(&entry.module, version);
1675-
if !manifest.patches.contains_key(&purl) {
1676-
continue;
1677-
}
1678-
// Explicit vendor entries take precedence over the synthesis
1679-
// (vendor may have taken over an apply redirect).
1680-
if socket_patch_core::vendor::lookup_entry(entries, &purl).is_some() {
1681-
continue;
1682-
}
16831695
// SECURITY: module/version come from a committed (tamper-able)
16841696
// go.mod and are about to key a path we hash. Apply the same
16851697
// fail-closed coordinate guard `go_redirect` itself uses before
16861698
// building the copy-dir path.
16871699
if !are_safe_redirect_coords(&entry.module, version) {
16881700
continue;
16891701
}
1690-
go_patches.insert(
1691-
purl,
1692-
copy_dir_for(&common.cwd, GO_PATCHES_DIR, &entry.module, version),
1693-
);
1702+
// go.mod spells `+incompatible` with a literal `+`; the API keys it
1703+
// `%2B` (#484). Match by release and key the copy by each record's
1704+
// own spelling, which is what verification looks up.
1705+
let Some(records) =
1706+
by_release.get(&PurlKey::new(&build_golang_purl(&entry.module, version)))
1707+
else {
1708+
continue;
1709+
};
1710+
for purl in records {
1711+
// Explicit vendor entries take precedence over the synthesis
1712+
// (vendor may have taken over an apply redirect).
1713+
if socket_patch_core::vendor::lookup_entry(entries, purl).is_some() {
1714+
continue;
1715+
}
1716+
go_patches.insert(
1717+
(*purl).clone(),
1718+
copy_dir_for(&common.cwd, GO_PATCHES_DIR, &entry.module, version),
1719+
);
1720+
}
16941721
}
16951722
go_patches
16961723
}

‎crates/socket-patch-cli/tests/covgap_commands_vex.rs‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1073,6 +1073,109 @@ fn go_patches_replace_for_an_unselected_version_is_not_attested() {
10731073
}
10741074
}
10751075

1076+
/// #484: the patch API keys a `+incompatible` Go module as
1077+
/// `@v2.0.0%2Bincompatible`, while go.mod spells the replace's version with
1078+
/// a literal `+`. vex must match the two spellings: an applied go-patches
1079+
/// redirect attests (it used to be omitted as `not_applied`, exit 1), while
1080+
/// a go.work replace that overrides it, or a require that selects another
1081+
/// version, is still caught, never attested, and reported under the
1082+
/// record's own `%2B` spelling.
1083+
#[test]
1084+
fn go_patches_replace_of_an_incompatible_module_matches_the_encoded_key() {
1085+
let module = "example.com/legacy";
1086+
let version = "v2.0.0+incompatible";
1087+
// (omission reason, if any)
1088+
for omitted in [
1089+
None,
1090+
Some("go_workspace_override"),
1091+
Some("go_version_not_selected"),
1092+
] {
1093+
let overridden = omitted == Some("go_workspace_override");
1094+
let required = if omitted == Some("go_version_not_selected") {
1095+
"v2.1.0+incompatible"
1096+
} else {
1097+
version
1098+
};
1099+
let tmp = tempfile::tempdir().unwrap();
1100+
let cwd = tmp.path();
1101+
std::fs::write(
1102+
cwd.join("go.mod"),
1103+
format!(
1104+
"module example.com/app\n\ngo 1.21\n\nrequire {module} {required}\n\n\
1105+
replace {module} {version} => ./.socket/go-patches/{module}@{version}\n"
1106+
),
1107+
)
1108+
.unwrap();
1109+
let go_work = cwd.join("go.work");
1110+
if overridden {
1111+
std::fs::write(
1112+
&go_work,
1113+
format!("go 1.21\n\nuse .\n\nreplace {module} {version} => ./fork\n"),
1114+
)
1115+
.unwrap();
1116+
}
1117+
let patched = b"package legacy // patched\n";
1118+
let copy = cwd.join(format!(".socket/go-patches/{module}@{version}"));
1119+
std::fs::create_dir_all(&copy).unwrap();
1120+
std::fs::write(copy.join("lib.go"), patched).unwrap();
1121+
let encoded = "pkg:golang/example.com/legacy@v2.0.0%2Bincompatible";
1122+
let mut manifest = PatchManifest::new();
1123+
manifest.patches.insert(
1124+
encoded.to_string(),
1125+
make_record(
1126+
GOOD_UUID,
1127+
"lib.go",
1128+
"a".repeat(64).as_str(),
1129+
&compute_git_sha256_from_bytes(patched),
1130+
"GHSA-go-incompatible",
1131+
&["CVE-2026-484"],
1132+
),
1133+
);
1134+
write_manifest(cwd, &manifest);
1135+
let empty_cache = cwd.join("empty-gomodcache");
1136+
std::fs::create_dir_all(&empty_cache).unwrap();
1137+
1138+
let out = cli()
1139+
.env("GOMODCACHE", &empty_cache)
1140+
.env(
1141+
"GOWORK",
1142+
if overridden {
1143+
go_work.as_os_str()
1144+
} else {
1145+
"off".as_ref()
1146+
},
1147+
)
1148+
.args([
1149+
"vex",
1150+
"--cwd",
1151+
cwd.to_str().unwrap(),
1152+
"--product",
1153+
"pkg:golang/example.com/app@v0.0.1",
1154+
])
1155+
.output()
1156+
.expect("invoke vex");
1157+
let stdout = String::from_utf8_lossy(&out.stdout);
1158+
let stderr = String::from_utf8_lossy(&out.stderr);
1159+
if let Some(reason) = omitted {
1160+
assert_eq!(out.status.code(), Some(1), "stderr:\n{stderr}");
1161+
assert!(!stdout.contains("not_affected"), "{stdout}");
1162+
assert!(
1163+
stderr.contains(&format!("omitting {encoded} from VEX"))
1164+
&& stderr.contains(&format!("({reason})")),
1165+
"stderr:\n{stderr}"
1166+
);
1167+
} else {
1168+
assert!(out.status.success(), "stderr:\n{stderr}");
1169+
let doc: Value = serde_json::from_str(&stdout).expect("VEX JSON on stdout");
1170+
let stmts = doc["statements"].as_array().unwrap();
1171+
assert_eq!(stmts.len(), 1, "{stdout}");
1172+
assert_eq!(stmts[0]["vulnerability"]["name"], "GHSA-go-incompatible");
1173+
assert_eq!(stmts[0]["status"], "not_affected");
1174+
assert_eq!(stmts[0]["products"][0]["subcomponents"][0]["@id"], encoded);
1175+
}
1176+
}
1177+
}
1178+
10761179
// ──────────────────────────────────────────────────────────────────────
10771180
// Corrupt vendor ledger on a MANIFEST-FREE project (the D2 vendored
10781181
// posture): the ledger was the only possible source of records, and it

0 commit comments

Comments
 (0)