security: fix 13 findings from security audit

cmd/admin/handlers/get.go

@@ -110,12 +110,15 @@ func (h *HandlersAdmin) CarvesDownloadHandler(w http.ResponseWriter, r *http.Req
 		log.Info().Msg("empty carve session")
 		return
 	}
-	// Check if carve is archived already
 	carve, err := h.Carves.GetBySession(carveSession)
 	if err != nil {
 		log.Err(err).Msgf("error getting carve")
 		return
 	}
+	if carve.EnvironmentID != env.ID {
+		log.Info().Msgf("carve env %d does not match requested env %d", carve.EnvironmentID, env.ID)
+		return
+	}
 	var archived *carves.CarveResult
 	if !carve.Archived {
 		archived, err = h.Carves.Archive(carveSession, h.CarvesFolder)

cmd/admin/oidc.go

@@ -207,6 +207,15 @@ func oidcCallbackHandler(w http.ResponseWriter, r *http.Request) {
 // if JITProvision is enabled; reject otherwise. Threat T16, T25.
 func resolveOIDCUser(identity auth.ResolvedIdentity) (users.AdminUser, error) {
 	if exists, existing := adminUsers.ExistsGet(identity.PreferredUsername); exists {
+		if existing.AuthSource == "" {
+			return users.AdminUser{}, fmt.Errorf("username %q is a local account and cannot be claimed by federated login", identity.PreferredUsername)
+		}
+		if existing.AuthSource != "oidc" {
+			if err := adminUsers.ChangeAuthSource(existing.Username, "oidc"); err != nil {
+				return users.AdminUser{}, fmt.Errorf("updating auth source: %w", err)
+			}
+			existing.AuthSource = "oidc"
+		}
 		return existing, nil
 	}
 	if flagParams == nil || flagParams.OIDC == nil || !flagParams.OIDC.JITProvision {
@@ -219,6 +228,7 @@ func resolveOIDCUser(identity auth.ResolvedIdentity) (users.AdminUser, error) {
 	if err != nil {
 		return users.AdminUser{}, fmt.Errorf("new user: %w", err)
 	}
+	u.AuthSource = "oidc"
 	if err := adminUsers.Create(u); err != nil {
 		return users.AdminUser{}, fmt.Errorf("create user: %w", err)
 	}

cmd/admin/templates/node.html

@@ -564,7 +564,7 @@ <h4 class="modal-title">Add / remove tags</h4>
               targets: 1,
               render: function (data, type, row, meta) {
                 if (type === 'display') {
-                  return '<pre>' + data + '</pre>';
+                  return '<pre>' + $('<div>').text(data).html() + '</pre>';
                 } else {
                   return data;
                 }

cmd/api/handlers/auth_logout.go

@@ -90,6 +90,7 @@ func (h *HandlersApi) LogoutHandler(w http.ResponseWriter, r *http.Request) {
 	// tenant URL + client_id without an actual session to terminate
 	// (pentest finding: unauthenticated IdP metadata disclosure).
 	var authenticated bool
+	var userAuthSource string
 	tokenCookie, err := r.Cookie("osctrl_token")
 	if err == nil && tokenCookie.Value != "" && len(h.JWTSecret) > 0 {
 		claims, valid := h.Users.CheckToken(string(h.JWTSecret), tokenCookie.Value)
@@ -100,6 +101,9 @@ func (h *HandlersApi) LogoutHandler(w http.ResponseWriter, r *http.Request) {
 				// client-side cookies and return the IdP URL.
 				log.Warn().Err(cerr).Str("user", claims.Username).Msg("logout: ClearToken failed")
 			}
+			if exists, u := h.Users.ExistsGet(claims.Username); exists {
+				userAuthSource = u.AuthSource
+			}
 		}
 	}
 
@@ -159,7 +163,8 @@ func (h *HandlersApi) LogoutHandler(w http.ResponseWriter, r *http.Request) {
 	// the OIDC callback (auth_oidc.go OIDCCallbackHandler). SAML and
 	// password flows never touch it. So:
 	//   id_token cookie present + valid token → OIDC session
-	//   id_token cookie absent → SAML or password session
+	//   id_token cookie absent + auth_source="saml" → SAML session
+	//   id_token cookie absent + auth_source="" → password session
 	//
 	// Anonymous callers always get the empty response (pentest
 	// T-IDP-DISCLOSURE: no IdP scrape without auth).
@@ -185,7 +190,7 @@ func (h *HandlersApi) LogoutHandler(w http.ResponseWriter, r *http.Request) {
 			resp.IdPLogoutURL = oidcProvider.EndSessionURL()
 			resp.IdPClientID = oidcClientID
 			resp.IdPIDTokenHint = idTokenHint
-		case !isOIDCSession && samlProvider != nil:
+		case !isOIDCSession && samlProvider != nil && userAuthSource == "saml":
 			resp.AuthSource = "saml"
 			if samlLogoutURL != "" {
 				resp.IdPLogoutURL = samlLogoutURL

cmd/api/handlers/auth_resolve.go

@@ -48,6 +48,18 @@ func (h *HandlersApi) resolveFederatedUser(identity auth.ResolvedIdentity, jitPr
 		return users.AdminUser{}, fmt.Errorf("%w: empty username", ErrAuthUserRejected)
 	}
 	if exists, existing := h.Users.ExistsGet(identity.PreferredUsername); exists {
+		if existing.AuthSource == "" {
+			return users.AdminUser{}, fmt.Errorf("%w: username %q is a local account and cannot be claimed by federated login",
+				ErrAuthUserRejected, identity.PreferredUsername)
+		}
+		// Allow cross-protocol federated login (oidc↔saml) — same IdP
+		// may serve both protocols. Update the stamp to the current one.
+		if existing.AuthSource != authSource {
+			if err := h.Users.ChangeAuthSource(existing.Username, authSource); err != nil {
+				return users.AdminUser{}, fmt.Errorf("%w: updating auth source: %v", ErrAuthUserRejected, err)
+			}
+			existing.AuthSource = authSource
+		}
 		return existing, nil
 	}
 	if !jitProvision {

cmd/api/handlers/carves.go

@@ -254,10 +254,9 @@ func (h *HandlersApi) CarvesRunHandler(w http.ResponseWriter, r *http.Request) {
 	// the splice site (single-quote escape, LIKE pattern escape) — no
 	// allowlist gate here so legitimate paths containing spaces or
 	// non-ASCII characters round-trip correctly.
-	// Make sure the user has permissions to run queries in the environments
 	for _, e := range c.Environments {
-		if !h.Users.CheckPermissions(ctx[ctxUser], users.QueryLevel, e) {
-			apiErrorResponse(w, fmt.Sprintf("%s has insufficient permissions to run queries in environment %s", ctx[ctxUser], e), http.StatusForbidden, nil)
+		if !h.Users.CheckPermissions(ctx[ctxUser], users.CarveLevel, e) {
+			apiErrorResponse(w, fmt.Sprintf("%s has insufficient permissions to run carves in environment %s", ctx[ctxUser], e), http.StatusForbidden, nil)
 			return
 		}
 	}

cmd/api/handlers/environments.go

@@ -169,25 +169,27 @@ func (h *HandlersApi) EnvironmentsHandler(w http.ResponseWriter, r *http.Request
 		apiErrorResponse(w, "error getting environments", http.StatusInternalServerError, err)
 		return
 	}
-	var out []environments.TLSEnvironment
+	var out []any
 	if h.Users.IsAdmin(requester) {
-		out = envAll
+		for _, e := range envAll {
+			out = append(out, e)
+		}
 	} else {
 		access, gerr := h.Users.GetAccess(requester)
 		if gerr != nil {
-			// Treat as "no access" and return [] — fail closed.
 			access = nil
 		}
 		for _, e := range envAll {
 			ea := access[e.UUID]
-			if ea.User || ea.Admin {
+			if ea.Admin {
 				out = append(out, e)
+			} else if ea.User {
+				out = append(out, projectEnvironmentView(e))
 			}
 		}
 	}
 	if out == nil {
-		// Marshal as [] not null for the SPA.
-		out = []environments.TLSEnvironment{}
+		out = []any{}
 	}
 	log.Debug().Msgf("Returned %d environment(s) to %s", len(out), requester)
 	h.AuditLog.Visit(requester, r.URL.Path, strings.Split(r.RemoteAddr, ":")[0], auditlog.NoEnvironment)

cmd/api/handlers/nodes.go

@@ -216,6 +216,10 @@ func (h *HandlersApi) DeleteNodeHandler(w http.ResponseWriter, r *http.Request)
 		apiErrorResponse(w, "error parsing POST body", http.StatusInternalServerError, err)
 		return
 	}
+	if _, err := h.Nodes.GetByUUIDEnv(n.UUID, env.ID); err != nil {
+		apiErrorResponse(w, "node not found", http.StatusNotFound, err)
+		return
+	}
 	if err := h.Nodes.ArchiveDeleteByUUID(n.UUID); err != nil {
 		if err.Error() == "record not found" {
 			apiErrorResponse(w, "node not found", http.StatusNotFound, err)

cmd/tls/handlers/post.go

@@ -5,6 +5,7 @@ import (
 	"compress/gzip"
 	"context"
 	"crypto/sha256"
+	"crypto/subtle"
 	"encoding/base64"
 	"encoding/json"
 	"errors"
@@ -789,6 +790,11 @@ func (h *HandlersTLS) CarveBlockHandler(w http.ResponseWriter, r *http.Request)
 	blockCarve := false
 	// Check if provided session_id matches with the request_id (carve query name)
 	if carve, err := h.Carves.GetCheckCarve(t.SessionID, t.RequestID); err == nil {
+		if carve.EnvironmentID != env.ID {
+			log.Warn().Msgf("CarveBlockHandler: carve env %d does not match URL env %d", carve.EnvironmentID, env.ID)
+			utils.HTTPResponse(w, utils.JSONApplicationUTF8, http.StatusOK, types.CarveBlockResponse{Success: false})
+			return
+		}
 		// Record ingested data
 		requestSize.WithLabelValues(string(env.UUID), "CarveBlock").Observe(float64(len(body)))
 		log.Info().Msgf("node %d in %s environment ingested %d bytes for CarveBlockHandler endpoint", carve.NodeID, env.Name, len(body))
@@ -1205,10 +1211,11 @@ func (h *HandlersTLS) OsqueryConfigEndpointHandler(w http.ResponseWriter, r *htt
 	confirmed := false
 	integrityCheck := false
 	for _, confEndpoint := range *h.ConfigEndpoints {
-		if confEndpoint.Environment == envVar && confEndpoint.Secret == secretVar {
+		envMatch := confEndpoint.Environment == envVar
+		secretMatch := subtle.ConstantTimeCompare([]byte(confEndpoint.Secret), []byte(secretVar)) == 1
+		if envMatch && secretMatch {
 			confirmed = true
 			integrityCheck = confEndpoint.IntegrityCheck
-			break
 		}
 	}
 	if !confirmed {

frontend/src/api/client.ts

@@ -312,16 +312,12 @@ export async function logout(): Promise<void> {
   if (idpLogoutUrl) {
     const postLogout = `${window.location.origin}/login`;
     const params = new URLSearchParams();
-
     if (authSource === 'saml') {
-      // SAML IdP logout (e.g. Auth0 /v2/logout) uses `returnTo`
-      // instead of the OIDC `post_logout_redirect_uri`.
       params.set('returnTo', postLogout);
       if (idpClientId) {
         params.set('client_id', idpClientId);
       }
     } else {
-      // OIDC RP-initiated logout — standard parameter names.
       params.set('post_logout_redirect_uri', postLogout);
       if (idpIdTokenHint) {
         params.set('id_token_hint', idpIdTokenHint);

pkg/auth/oidc/claims.go

@@ -17,6 +17,7 @@ type idTokenClaims struct {
 	Subject           string `json:"sub"`
 	PreferredUsername string `json:"preferred_username"`
 	Email             string `json:"email"`
+	EmailVerified     *bool  `json:"email_verified"`
 	Name              string `json:"name"`
 	GivenName         string `json:"given_name"`
 	FamilyName        string `json:"family_name"`
@@ -68,9 +69,13 @@ func pickUsername(c idTokenClaims, raw map[string]any, claim string) string {
 			return c.PreferredUsername
 		}
 	case "email":
-		if c.Email != "" {
+		if c.Email != "" && c.EmailVerified != nil && *c.EmailVerified {
 			return c.Email
 		}
+		if c.Email != "" && (c.EmailVerified == nil || !*c.EmailVerified) {
+			log.Warn().Msgf("oidc: email claim %q used as username but email_verified is not true — rejecting", c.Email)
+			return c.Subject
+		}
 	case "sub":
 		return c.Subject
 	}

pkg/auth/oidc/claims_test.go

@@ -9,11 +9,14 @@ import (
 // always-available fallback per OIDC spec; tests verify that the
 // pickUsername logic prefers the configured claim when present and
 // falls back to subject otherwise.
+func boolPtr(b bool) *bool { return &b }
+
 func TestPickUsername(t *testing.T) {
 	claims := idTokenClaims{
 		Subject:           "sub-uuid-1234",
 		PreferredUsername: "alice",
 		Email:             "alice@example.com",
+		EmailVerified:     boolPtr(true),
 		Name:              "Alice Tester",
 		GivenName:         "Alice",
 		FamilyName:        "Tester",
@@ -42,6 +45,28 @@ func TestPickUsername(t *testing.T) {
 	}
 }
 
+// TestPickUsernameUnverifiedEmail — when email_verified is false or
+// nil, the email claim must NOT be used as a username (audit finding 5).
+func TestPickUsernameUnverifiedEmail(t *testing.T) {
+	unverified := idTokenClaims{
+		Subject:       "sub-uuid-1234",
+		Email:         "alice@example.com",
+		EmailVerified: boolPtr(false),
+	}
+	got := pickUsername(unverified, nil, "email")
+	if got != "sub-uuid-1234" {
+		t.Fatalf("unverified email should fall back to sub, got %q", got)
+	}
+	nilVerified := idTokenClaims{
+		Subject: "sub-uuid-1234",
+		Email:   "alice@example.com",
+	}
+	got = pickUsername(nilVerified, nil, "email")
+	if got != "sub-uuid-1234" {
+		t.Fatalf("nil email_verified should fall back to sub, got %q", got)
+	}
+}
+
 // TestPickUsernameAbsentClaim — when the configured claim isn't on
 // the id_token, we fall back to subject. Test by clearing
 // PreferredUsername and asking for it.

pkg/carves/s3.go

@@ -134,6 +134,9 @@ func (carveS3 *CarverS3) Archive(carve CarvedFile, blocks []CarvedBlock) (*Carve
 	}
 	var parts []awsTypes.CompletedPart
 	for _, b := range blocks {
+		if b.BlockID < 0 || b.BlockID > 9998 {
+			return nil, fmt.Errorf("block_id %d out of valid range [0, 9998]", b.BlockID)
+		}
 		etag, err := carveS3.Concatenate(S3URLtoKey(b.Data, carveS3.S3Config.Bucket), fkey, b.BlockID+1, uploadOutput.UploadId)
 		if err != nil {
 			return nil, fmt.Errorf("error concatenating - %w", err)

pkg/carves/utils.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"encoding/base64"
 	"fmt"
+	"path/filepath"
 	"strings"
 
 	"github.com/jmpsec/osctrl/pkg/utils"
@@ -50,7 +51,7 @@ func S3URLtoKey(s3url, bucket string) string {
 // Function to generate a local file for carve archives
 func GenerateArchiveName(carve CarvedFile) string {
 	cPath := strings.ReplaceAll(strings.ReplaceAll(carve.Path, "/", "-"), "\\", "-")
-	return fmt.Sprintf(LocalFile, carve.UUID, carve.SessionID, cPath)
+	return fmt.Sprintf(LocalFile, filepath.Base(carve.UUID), filepath.Base(carve.SessionID), filepath.Base(cPath))
 }
 
 // Function to check if data is compressed using zstd

pkg/config/types.go

@@ -266,7 +266,7 @@ type YAMLConfigurationSAML struct {
 	MetaDataURL  string `yaml:"metadataUrl"`
 	RootURL      string `yaml:"rootUrl"`
 	LoginURL     string `yaml:"loginUrl"`
-	LogoutURL    string `yaml:"logoutUrl"`
+	LogoutURL    string `yaml:"logoutUrl"       mapstructure:"logoutUrl"`
 	JITProvision bool   `yaml:"jitProvision"   mapstructure:"jitProvision"`
 	// UsernameAttribute names the SAML attribute (by Name or
 	// FriendlyName) whose value becomes the osctrl username.

pkg/logging/process.go

@@ -55,10 +55,12 @@ func (l *LoggerTLS) ProcessLogQueryResult(queriesWrite types.QueryWriteRequest,
 	node, err := l.Nodes.GetByKey(queriesWrite.NodeKey)
 	if err != nil {
 		log.Err(err).Msg("error retrieving node")
+		return
 	}
-	// Integrity check
+	// Integrity check — hard reject on env mismatch
 	if envid != node.EnvironmentID {
-		log.Error().Msgf("ProcessLogQueryResult: EnvID[%d] does not match Node.EnvironmentID[%d]", envid, node.EnvironmentID)
+		log.Error().Msgf("ProcessLogQueryResult: EnvID[%d] does not match Node.EnvironmentID[%d] — dropping results", envid, node.EnvironmentID)
+		return
 	}
 	// Tap into results so we can update internal metrics
 	for q, r := range queriesWrite.Queries {

pkg/users/users.go

@@ -158,6 +158,12 @@ func (m *UserManager) HashPasswordWithSalt(password string) (string, error) {
 // failure is non-fatal — login succeeds even if the rehash write
 // fails (next login retries).
 func (m *UserManager) CheckLoginCredentials(username, password string) (bool, AdminUser) {
+	if password == "" {
+		if dummyHash != nil {
+			_ = bcrypt.CompareHashAndPassword(dummyHash, []byte(password))
+		}
+		return false, AdminUser{}
+	}
 	// Check if we should include service users
 	user, err := m.Get(username)
 	if err != nil {
@@ -309,9 +315,23 @@ func (m *UserManager) Create(user AdminUser) error {
 // New empty user
 func (m *UserManager) New(username, password, email, fullname string, admin, service bool) (AdminUser, error) {
 	if !m.Exists(username) {
-		passhash, err := m.HashPasswordWithSalt(password)
-		if err != nil {
-			return AdminUser{}, err
+		var passhash string
+		if password == "" {
+			randomBytes := make([]byte, 32)
+			if _, err := cryptorand.Read(randomBytes); err != nil {
+				return AdminUser{}, fmt.Errorf("generate random token: %w", err)
+			}
+			h, err := m.HashPasswordWithSalt(hex.EncodeToString(randomBytes))
+			if err != nil {
+				return AdminUser{}, err
+			}
+			passhash = h
+		} else {
+			h, err := m.HashPasswordWithSalt(password)
+			if err != nil {
+				return AdminUser{}, err
+			}
+			passhash = h
 		}
 		return AdminUser{
 			Username: username,
@@ -389,6 +409,20 @@ func (m *UserManager) ChangeService(username string, service bool) error {
 	return nil
 }
 
+// ChangeAuthSource to modify the auth_source for a user
+func (m *UserManager) ChangeAuthSource(username, authSource string) error {
+	user, err := m.Get(username)
+	if err != nil {
+		return fmt.Errorf("error getting user %w", err)
+	}
+	if authSource != user.AuthSource {
+		if err := m.DB.Model(&user).Updates(map[string]interface{}{"auth_source": authSource}).Error; err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 // All get all users
 func (m *UserManager) All() ([]AdminUser, error) {
 	var users []AdminUser