Skip to content

Commit acb5ee6

Browse files
committed
fix(updatecheck): verify rpm ownership and detect tray-staged DMG core (Spec 079 US2)
Cross-model review round 1 fixes: - rpm channel: the RPM database existing only proves the host is RPM-based; a manual tarball copy to /usr/bin/mcpproxy on Fedora was misclassified as rpm and shown a wrong 'sudo dnf upgrade' command. Ownership is now confirmed with a one-shot 'rpm -qf /usr/bin/mcpproxy' probe (3s timeout, fails closed to unknown per FR-009). - dmg channel: DMG installs run the core from the tray-staged ~/Library/Application Support/mcpproxy/bin/mcpproxy (and the bundled source lives under Contents/Resources/bin), so the .app/Contents/MacOS check never matched the process serving /api/v1/info and DMG installs reported unknown. Both paths now classify as dmg; that staged directory is written only by the tray's bundle-staging, so it uniquely implies a DMG install.
1 parent 52b14e1 commit acb5ee6

3 files changed

Lines changed: 123 additions & 15 deletions

File tree

docs/features/version-updates.md

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -181,12 +181,17 @@ in decreasing confidence order:
181181
`/home/linuxbrew/.linuxbrew`).
182182
2. **Docker**: `/.dockerenv` exists.
183183
3. **deb / rpm**: on Linux, the executable is exactly `/usr/bin/mcpproxy`
184-
**and** the owning package manager's database confirms it
185-
(`/var/lib/dpkg/info/mcpproxy.list` for deb; an rpm database for rpm).
186-
Both signals are required — a binary merely copied to `/usr/bin` (e.g. an
187-
AUR or manual install) stays `unknown`.
188-
4. **DMG**: on macOS, the executable runs from an `.app/Contents/MacOS`
189-
bundle.
184+
**and** the owning package manager confirms it owns the file
185+
(`/var/lib/dpkg/info/mcpproxy.list` for deb; for rpm, an rpm database
186+
exists **and** a one-shot `rpm -qf /usr/bin/mcpproxy` query names the
187+
`mcpproxy` package). Both signals are required — a binary merely copied to
188+
`/usr/bin` (e.g. an AUR or manual install), even on an RPM-based distro,
189+
stays `unknown`.
190+
4. **DMG**: on macOS, the executable runs from an `.app/Contents/MacOS` or
191+
`.app/Contents/Resources/bin` bundle path, or is the tray-staged core at
192+
`~/Library/Application Support/mcpproxy/bin/mcpproxy` (the process that
193+
actually serves the API for DMG installs; only the tray's bundle-staging
194+
writes that directory).
190195
5. **go install**: the Go toolchain stamped a real module version into the
191196
binary's build info while no release version was stamped via ldflags.
192197
6. Otherwise the channel is **`unknown`**.

internal/updatecheck/channel.go

Lines changed: 52 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
11
package updatecheck
22

33
import (
4+
"context"
45
"os"
6+
"os/exec"
57
"path/filepath"
68
"runtime"
79
"runtime/debug"
810
"strings"
11+
"time"
912

1013
"golang.org/x/mod/semver"
1114
)
@@ -70,6 +73,10 @@ type channelDetector struct {
7073
evalSymlinks func(string) (string, error)
7174
statFile func(string) error // nil error ⇒ the path exists
7275
readBuildInfo func() (*debug.BuildInfo, bool)
76+
// rpmOwnsBinary reports whether the mcpproxy RPM package owns
77+
// /usr/bin/mcpproxy (see rpmOwnsUsrBinMcpproxy). Only consulted on Linux
78+
// when the binary runs from /usr/bin/mcpproxy and an RPM database exists.
79+
rpmOwnsBinary func() bool
7380
}
7481

7582
func newChannelDetector(ldflagsVersion string) *channelDetector {
@@ -84,9 +91,31 @@ func newChannelDetector(ldflagsVersion string) *channelDetector {
8491
return err
8592
},
8693
readBuildInfo: debug.ReadBuildInfo,
94+
rpmOwnsBinary: rpmOwnsUsrBinMcpproxy,
8795
}
8896
}
8997

98+
// rpmProbeTimeout bounds the one-shot `rpm -qf` ownership query run at
99+
// startup on RPM-based hosts. rpm answers from a local database, so 3s is
100+
// generous; on expiry the probe fails closed to ChannelUnknown.
101+
const rpmProbeTimeout = 3 * time.Second
102+
103+
// rpmOwnsUsrBinMcpproxy reports whether the mcpproxy RPM package owns
104+
// /usr/bin/mcpproxy. The RPM database existing (checked by the caller) only
105+
// proves the host is RPM-based — a manual tarball copy to /usr/bin/mcpproxy on
106+
// Fedora would otherwise be misclassified as an rpm install and shown a wrong
107+
// `dnf upgrade` command (FR-009: never guess). Any failure — rpm binary
108+
// missing, file unowned, timeout — degrades to false, i.e. ChannelUnknown.
109+
func rpmOwnsUsrBinMcpproxy() bool {
110+
ctx, cancel := context.WithTimeout(context.Background(), rpmProbeTimeout)
111+
defer cancel()
112+
out, err := exec.CommandContext(ctx, "rpm", "-qf", "--qf", "%{NAME}", "/usr/bin/mcpproxy").Output()
113+
if err != nil {
114+
return false
115+
}
116+
return strings.TrimSpace(string(out)) == "mcpproxy"
117+
}
118+
90119
// DetectChannel identifies the install channel of the running binary.
91120
// ldflagsVersion is the build version handed to the checker (see
92121
// channelDetector.ldflagsVersion). The build-time marker wins; heuristics
@@ -124,14 +153,19 @@ func (d *channelDetector) detect() string {
124153
}
125154

126155
// (3) deb/rpm: require BOTH the package-manager-owned install path AND
127-
// that package manager's database evidence. AUR/manual installs share
128-
// /usr/bin/mcpproxy but have neither DB entry and must fall to unknown;
129-
// both DBs matching is ambiguous and also falls to unknown. This branch
130-
// is terminal for the /usr/bin/mcpproxy path: never guess (FR-009).
156+
// package-specific ownership evidence. dpkg keeps a per-package file list
157+
// (mcpproxy.list); rpm has no per-package file to stat, so the database
158+
// presence (host is RPM-based) is confirmed with an `rpm -qf` ownership
159+
// query — the DB alone would misclassify a manual tarball copy to
160+
// /usr/bin/mcpproxy on any RPM distro. AUR/manual installs have neither
161+
// and must fall to unknown; both matching is ambiguous and also falls to
162+
// unknown. This branch is terminal for the /usr/bin/mcpproxy path: never
163+
// guess (FR-009).
131164
if d.goos == "linux" && path == "/usr/bin/mcpproxy" {
132165
hasDeb := d.statFile("/var/lib/dpkg/info/mcpproxy.list") == nil
133-
hasRPM := d.statFile("/var/lib/rpm/rpmdb.sqlite") == nil ||
166+
rpmDBPresent := d.statFile("/var/lib/rpm/rpmdb.sqlite") == nil ||
134167
d.statFile("/var/lib/rpm/Packages") == nil
168+
hasRPM := rpmDBPresent && d.rpmOwnsBinary()
135169
switch {
136170
case hasDeb && !hasRPM:
137171
return ChannelDeb
@@ -142,8 +176,19 @@ func (d *channelDetector) detect() string {
142176
}
143177
}
144178

145-
// (4) macOS app bundle ⇒ DMG install.
146-
if d.goos == "darwin" && strings.Contains(path, ".app/Contents/MacOS") {
179+
// (4) macOS DMG install. Three shapes, because the tray stages the
180+
// bundled core out of the app bundle before running it
181+
// (cmd/mcpproxy-tray stageBundledCore):
182+
// - .app/Contents/MacOS — the tray itself / direct bundle exec
183+
// - .app/Contents/Resources/bin — the bundled core run in place
184+
// - ~/Library/Application Support/mcpproxy/bin/mcpproxy — the staged
185+
// core, which is the process that actually serves /api/v1/info for
186+
// DMG installs; that directory is written only by the tray's
187+
// bundle-staging path, so it uniquely implies a DMG install.
188+
if d.goos == "darwin" &&
189+
(strings.Contains(path, ".app/Contents/MacOS") ||
190+
strings.Contains(path, ".app/Contents/Resources/bin") ||
191+
strings.HasSuffix(path, "/Library/Application Support/mcpproxy/bin/mcpproxy")) {
147192
return ChannelDMG
148193
}
149194

internal/updatecheck/channel_test.go

Lines changed: 60 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ func testDetector() *channelDetector {
1818
evalSymlinks: func(p string) (string, error) { return p, nil },
1919
statFile: func(string) error { return os.ErrNotExist },
2020
readBuildInfo: func() (*debug.BuildInfo, bool) { return nil, false },
21+
rpmOwnsBinary: func() bool { return false },
2122
}
2223
}
2324

@@ -134,7 +135,7 @@ func TestDetectChannel_DebRequiresBothSignals(t *testing.T) {
134135
}
135136

136137
func TestDetectChannel_RPMRequiresBothSignals(t *testing.T) {
137-
t.Run("path + rpmdb.sqlite -> rpm", func(t *testing.T) {
138+
t.Run("path + rpmdb.sqlite + rpm ownership -> rpm", func(t *testing.T) {
138139
d := testDetector()
139140
d.execPath = func() (string, error) { return "/usr/bin/mcpproxy", nil }
140141
d.statFile = func(p string) error {
@@ -143,12 +144,13 @@ func TestDetectChannel_RPMRequiresBothSignals(t *testing.T) {
143144
}
144145
return os.ErrNotExist
145146
}
147+
d.rpmOwnsBinary = func() bool { return true }
146148
if got := d.detect(); got != ChannelRPM {
147149
t.Errorf("detect() = %q, want %q", got, ChannelRPM)
148150
}
149151
})
150152

151-
t.Run("path + legacy Packages db -> rpm", func(t *testing.T) {
153+
t.Run("path + legacy Packages db + rpm ownership -> rpm", func(t *testing.T) {
152154
d := testDetector()
153155
d.execPath = func() (string, error) { return "/usr/bin/mcpproxy", nil }
154156
d.statFile = func(p string) error {
@@ -157,11 +159,30 @@ func TestDetectChannel_RPMRequiresBothSignals(t *testing.T) {
157159
}
158160
return os.ErrNotExist
159161
}
162+
d.rpmOwnsBinary = func() bool { return true }
160163
if got := d.detect(); got != ChannelRPM {
161164
t.Errorf("detect() = %q, want %q", got, ChannelRPM)
162165
}
163166
})
164167

168+
t.Run("rpm db present but package does not own the binary -> unknown", func(t *testing.T) {
169+
// Manual tarball copy to /usr/bin/mcpproxy on Fedora: the host RPM
170+
// database exists, but `rpm -qf` disowns the file. Suggesting
171+
// `dnf upgrade mcpproxy` here would be wrong (FR-009).
172+
d := testDetector()
173+
d.execPath = func() (string, error) { return "/usr/bin/mcpproxy", nil }
174+
d.statFile = func(p string) error {
175+
if p == "/var/lib/rpm/rpmdb.sqlite" {
176+
return nil
177+
}
178+
return os.ErrNotExist
179+
}
180+
d.rpmOwnsBinary = func() bool { return false }
181+
if got := d.detect(); got != ChannelUnknown {
182+
t.Errorf("detect() = %q, want %q", got, ChannelUnknown)
183+
}
184+
})
185+
165186
t.Run("both dpkg and rpm evidence is ambiguous -> unknown", func(t *testing.T) {
166187
d := testDetector()
167188
d.execPath = func() (string, error) { return "/usr/bin/mcpproxy", nil }
@@ -174,6 +195,7 @@ func TestDetectChannel_RPMRequiresBothSignals(t *testing.T) {
174195
}
175196
return nil
176197
}
198+
d.rpmOwnsBinary = func() bool { return true }
177199
if got := d.detect(); got != ChannelUnknown {
178200
t.Errorf("detect() = %q, want %q", got, ChannelUnknown)
179201
}
@@ -190,6 +212,42 @@ func TestDetectChannel_DMG(t *testing.T) {
190212
t.Errorf("detect() = %q, want %q", got, ChannelDMG)
191213
}
192214

215+
t.Run("tray-staged core path -> dmg", func(t *testing.T) {
216+
// DMG reality: the tray stages the bundled core into Application
217+
// Support and runs it from there — that process is the one serving
218+
// /api/v1/info, so it must still classify as dmg.
219+
d := testDetector()
220+
d.goos = "darwin"
221+
d.execPath = func() (string, error) {
222+
return "/Users/dev/Library/Application Support/mcpproxy/bin/mcpproxy", nil
223+
}
224+
if got := d.detect(); got != ChannelDMG {
225+
t.Errorf("detect() = %q, want %q", got, ChannelDMG)
226+
}
227+
})
228+
229+
t.Run("bundled core under Resources/bin -> dmg", func(t *testing.T) {
230+
d := testDetector()
231+
d.goos = "darwin"
232+
d.execPath = func() (string, error) {
233+
return "/Applications/MCPProxy.app/Contents/Resources/bin/mcpproxy", nil
234+
}
235+
if got := d.detect(); got != ChannelDMG {
236+
t.Errorf("detect() = %q, want %q", got, ChannelDMG)
237+
}
238+
})
239+
240+
t.Run("staged-core path on linux is not dmg", func(t *testing.T) {
241+
d := testDetector()
242+
d.goos = "linux"
243+
d.execPath = func() (string, error) {
244+
return "/home/dev/Library/Application Support/mcpproxy/bin/mcpproxy", nil
245+
}
246+
if got := d.detect(); got != ChannelUnknown {
247+
t.Errorf("detect() = %q, want %q", got, ChannelUnknown)
248+
}
249+
})
250+
193251
t.Run("app bundle path on linux is not dmg", func(t *testing.T) {
194252
d := testDetector()
195253
d.goos = "linux"

0 commit comments

Comments
 (0)