CP: Prevent script only same hash uploads (#35210)

CP of #35188
This commit is contained in:
Carlo
2025-11-05 11:40:44 -05:00
committed by GitHub
parent 35e35cb1a4
commit ea0f99b6f8
2 changed files with 116 additions and 0 deletions
@@ -245,6 +245,36 @@ func (ds *Datastore) MatchOrCreateSoftwareInstaller(ctx context.Context, payload
}
}
// Enforce team-scoped uniqueness by storage hash, aligning upload behavior with GitOps.
// However, if the duplicate-by-hash is for the same title/source on the same team,
// let the DB unique (team,title) constraint surface the conflict (so tests expecting
// a 409 Conflict with "already exists" still pass).
// Only validate for script packages (.sh/.ps1) where content hash equals functionality.
// Binary installers can legitimately share content with different install scripts.
if payload.StorageID != "" && fleet.IsScriptPackage(payload.Extension) {
var tmID uint
if payload.TeamID != nil {
tmID = *payload.TeamID
}
// Check duplicates by content hash only (ignore URL) to align with GitOps/apply rules.
teamsByHash, err := ds.GetTeamsWithInstallerByHash(ctx, payload.StorageID, "")
if err != nil {
return 0, 0, ctxerr.Wrap(ctx, err, "check duplicate installer by hash")
}
if found, exists := teamsByHash[tmID]; exists {
// If the existing installer has the same title and source, allow the insert to proceed
// so that the existing UNIQUE (global_or_team_id, title_id) constraint yields a
// Conflict error with the expected message.
if !(found.Title == payload.Title && found.Source == payload.Source) {
return 0, 0, fleet.NewInvalidArgumentError(
"software",
"Couldn't add software. An installer with identical contents already exists on this team.",
)
}
// If exact duplicate (same title and source), continue to let DB constraint handle it
}
}
if err := ds.addSoftwareTitleToMatchingSoftware(ctx, titleID, payload); err != nil {
return 0, 0, ctxerr.Wrap(ctx, err, "add software title to matching software")
}
@@ -4,6 +4,7 @@ import (
"bytes"
"context"
"database/sql"
"errors"
"fmt"
"os"
"path/filepath"
@@ -47,6 +48,7 @@ func TestSoftwareInstallers(t *testing.T) {
{"MatchOrCreateSoftwareInstallerWithAutomaticPolicies", testMatchOrCreateSoftwareInstallerWithAutomaticPolicies},
{"GetDetailsForUninstallFromExecutionID", testGetDetailsForUninstallFromExecutionID},
{"GetTeamsWithInstallerByHash", testGetTeamsWithInstallerByHash},
{"MatchOrCreateSoftwareInstallerDuplicateHash", testMatchOrCreateSoftwareInstallerDuplicateHash},
{"BatchSetSoftwareInstallersSetupExperienceSideEffects", testBatchSetSoftwareInstallersSetupExperienceSideEffects},
{"EditDeleteSoftwareInstallersActivateNextActivity", testEditDeleteSoftwareInstallersActivateNextActivity},
{"BatchSetSoftwareInstallersActivateNextActivity", testBatchSetSoftwareInstallersActivateNextActivity},
@@ -3619,3 +3621,87 @@ func testSoftwareTitleDisplayName(t *testing.T, ds *Datastore) {
assert.Empty(t, software.DisplayName)
}
func testMatchOrCreateSoftwareInstallerDuplicateHash(t *testing.T, ds *Datastore) {
ctx := context.Background()
user := test.NewUser(t, ds, "Alice", "alice@example.com", true)
teamA, err := ds.NewTeam(ctx, &fleet.Team{Name: "Team A"})
require.NoError(t, err)
teamB, err := ds.NewTeam(ctx, &fleet.Team{Name: "Team B"})
require.NoError(t, err)
const sameHash = "dup-hash-001"
mkPayload := func(teamID *uint, filename, title string) *fleet.UploadSoftwareInstallerPayload {
tfr, err := fleet.NewTempFileReader(strings.NewReader("same-bytes"), t.TempDir)
require.NoError(t, err)
return &fleet.UploadSoftwareInstallerPayload{
InstallerFile: tfr,
Extension: "sh",
StorageID: sameHash,
Filename: filename,
Title: title,
Version: "1.0",
Source: "apps",
Platform: "darwin",
UserID: user.ID,
ValidatedLabels: &fleet.LabelIdentsWithScope{},
TeamID: teamID,
}
}
// Create on Team A → success
_, _, err = ds.MatchOrCreateSoftwareInstaller(ctx, mkPayload(&teamA.ID, "a.sh", "title-a"))
require.NoError(t, err)
// Duplicate on Team A with different name/title but same hash → reject
_, _, err = ds.MatchOrCreateSoftwareInstaller(ctx, mkPayload(&teamA.ID, "b.sh", "title-b"))
require.Error(t, err)
var iae *fleet.InvalidArgumentError
if !errors.As(err, &iae) {
t.Fatalf("expected InvalidArgumentError for same-team duplicate hash, got: %T: %v", err, err)
}
// Same hash on different team → allowed
_, _, err = ds.MatchOrCreateSoftwareInstaller(ctx, mkPayload(&teamB.ID, "c.sh", "title-c"))
require.NoError(t, err)
// Global scope first time → allowed
_, _, err = ds.MatchOrCreateSoftwareInstaller(ctx, mkPayload(nil, "global1.sh", "title-g1"))
require.NoError(t, err)
// Global scope second time (duplicate hash) → reject
_, _, err = ds.MatchOrCreateSoftwareInstaller(ctx, mkPayload(nil, "global2.sh", "title-g2"))
require.Error(t, err)
var iae2 *fleet.InvalidArgumentError
if !errors.As(err, &iae2) {
t.Fatalf("expected InvalidArgumentError for global duplicate hash, got: %T: %v", err, err)
}
// Test that binary packages (.pkg) with duplicate hash ARE allowed
mkPkgPayload := func(teamID *uint, filename, title string) *fleet.UploadSoftwareInstallerPayload {
tfr, err := fleet.NewTempFileReader(strings.NewReader("same-binary-bytes"), t.TempDir)
require.NoError(t, err)
return &fleet.UploadSoftwareInstallerPayload{
InstallerFile: tfr,
Extension: "pkg",
StorageID: "same-pkg-hash",
Filename: filename,
Title: title,
Version: "1.0",
Source: "apps",
Platform: "darwin",
UserID: user.ID,
ValidatedLabels: &fleet.LabelIdentsWithScope{},
TeamID: teamID,
}
}
// Binary packages with same hash on same team → allowed
_, _, err = ds.MatchOrCreateSoftwareInstaller(ctx, mkPkgPayload(&teamA.ID, "pkg1.pkg", "title-pkg1"))
require.NoError(t, err)
_, _, err = ds.MatchOrCreateSoftwareInstaller(ctx, mkPkgPayload(&teamA.ID, "pkg2.pkg", "title-pkg2"))
require.NoError(t, err, "binary packages with same hash should be allowed on same team")
}