fix(auth): complete Task 13 deployment review

This commit is contained in:
2026-08-17 22:15:41 +02:00
parent 7e52df2702
commit 0d0c15b4b8
11 changed files with 228 additions and 50 deletions
+9 -24
View File
@@ -8,33 +8,18 @@ import { expandLocalHome, localPrincipal, upstreamPrincipal } from "../src/auth/
import { buildApp } from "../src/app.js"; import { buildApp } from "../src/app.js";
import { loadConfig } from "../src/config.js"; import { loadConfig } from "../src/config.js";
test("server smoke trusted claims transform through nginx to a non-admin principal", () => { test("server smoke rejects retired trusted claims under OIDC authentication", () => {
const smoke = readFileSync("../scripts/unified-deployment-smoke.sh", "utf8"); const smoke = readFileSync("../scripts/unified-deployment-smoke.sh", "utf8");
const nginx = readFileSync("../docker/nginx.conf.template", "utf8"); for (const header of [
const helper = smoke.match(/task13_server_auth_headers\(\) \{([\s\S]*?)\n\}/)?.[1] ?? ""; "x-thoth-trusted-principal-issuer",
const trusted = Object.fromEntries( "x-thoth-trusted-principal-subject",
[...helper.matchAll(/-H '([^:']+): ([^']+)'/g)].map((match) => [match[1].toLowerCase(), match[2]]), "x-thoth-trusted-principal-display-name",
); "x-thoth-trusted-is-admin",
const normalized: Record<string, string> = {};
for (const [header, suffix] of [
["x-thoth-principal-issuer", "principal_issuer"],
["x-thoth-principal-subject", "principal_subject"],
["x-thoth-principal-display-name", "principal_display_name"],
["x-thoth-is-admin", "is_admin"],
]) { ]) {
expect(nginx).toContain(`$http_x_thoth_trusted_${suffix}`); expect(smoke).toContain(`-H '${header}:`);
const value = trusted[`x-thoth-trusted-${header.slice("x-thoth-".length)}`];
if (value !== undefined) normalized[header] = value;
} }
expect(smoke).toContain('[[ "$trusted_header_status" == 401 ]]');
expect(upstreamPrincipal(normalized)).toEqual({ expect(smoke).toContain("server accepted retired trusted identity headers");
issuer: "task13-proxy",
subject: "task13-user",
displayName: "Task 13 User",
roles: ["user"],
permissions: ["session.use"],
isAdmin: false,
});
}); });
test("local mode resolves a stable local principal", async () => { test("local mode resolves a stable local principal", async () => {
+11 -1
View File
@@ -68,11 +68,20 @@ for (const serviceName of ["embedding", "embedding-model-init"]) {
throw new Error(`${serviceName} image must be pinned by version and digest`); throw new Error(`${serviceName} image must be pinned by version and digest`);
} }
} }
for (const serviceName of ["qdrant", "embedding", "embedding-model-init"]) { for (const serviceName of ["embedding", "embedding-model-init"]) {
if ((config.services[serviceName].ports || []).length !== 0) { if ((config.services[serviceName].ports || []).length !== 0) {
throw new Error(`${serviceName} must not publish a host port`); throw new Error(`${serviceName} must not publish a host port`);
} }
} }
const qdrantPorts = config.services.qdrant.ports || [];
if (profile === "local") {
if (qdrantPorts.length !== 1 || qdrantPorts[0].host_ip !== "127.0.0.1"
|| Number(qdrantPorts[0].published) !== 6333 || Number(qdrantPorts[0].target) !== 6333) {
throw new Error("local qdrant may publish only 127.0.0.1:6333");
}
} else if (qdrantPorts.length !== 0) {
throw new Error("server qdrant must not publish a host port");
}
if ((config.services.qdrant.expose || []).join(",") !== "6333") throw new Error("qdrant must expose only 6333"); if ((config.services.qdrant.expose || []).join(",") !== "6333") throw new Error("qdrant must expose only 6333");
if ((config.services.embedding.expose || []).join(",") !== "11434") throw new Error("embedding must expose only 11434"); if ((config.services.embedding.expose || []).join(",") !== "11434") throw new Error("embedding must expose only 11434");
if (!config.services.qdrant.healthcheck) throw new Error("qdrant must define a healthcheck"); if (!config.services.qdrant.healthcheck) throw new Error("qdrant must define a healthcheck");
@@ -155,6 +164,7 @@ assert_remote_required() {
THT_WORKSPACE_GIT_REMOTE=https://git.example.invalid/platform/thoth-workspaces.git \ THT_WORKSPACE_GIT_REMOTE=https://git.example.invalid/platform/thoth-workspaces.git \
PI_AUTH_FILE=/dev/null \ PI_AUTH_FILE=/dev/null \
THT_SECRETS_FILE=/dev/null \ THT_SECRETS_FILE=/dev/null \
THT_AUTH_CONFIG_ROOT=/tmp/thothii-auth \
docker compose -f compose.yaml config --format json >"$tmp/base.json" docker compose -f compose.yaml config --format json >"$tmp/base.json"
node - "$tmp/base.json" <<'NODE' node - "$tmp/base.json" <<'NODE'
const fs = require("fs"); const fs = require("fs");
+3 -3
View File
@@ -428,7 +428,7 @@ EOF
chmod 0600 "$TASK13_OIDC_KEY" chmod 0600 "$TASK13_OIDC_KEY"
chmod 0644 "$TASK13_OIDC_CERT" chmod 0644 "$TASK13_OIDC_CERT"
cat >"$TASK13_OIDC_SERVER" <<'EOF' cat >"$TASK13_OIDC_SERVER" <<'EOF'
import { createPublicKey, generateKeyPairSync } from "node:crypto"; import { generateKeyPairSync } from "node:crypto";
import { readFileSync } from "node:fs"; import { readFileSync } from "node:fs";
import https from "node:https"; import https from "node:https";
@@ -436,7 +436,7 @@ const origin = "https://task13-fake-oidc:9443";
const issuer = `${origin}/application/o/task13/`; const issuer = `${origin}/application/o/task13/`;
const expectedToken = process.env.TASK13_AUTHENTIK_API_TOKEN; const expectedToken = process.env.TASK13_AUTHENTIK_API_TOKEN;
const { publicKey } = generateKeyPairSync("rsa", { modulusLength: 2048 }); const { publicKey } = generateKeyPairSync("rsa", { modulusLength: 2048 });
const jwk = { ...createPublicKey(publicKey).export({ format: "jwk" }), kid: "task13", use: "sig", alg: "RS256" }; const jwk = { ...publicKey.export({ format: "jwk" }), kid: "task13", use: "sig", alg: "RS256" };
const send = (response, status, body) => { const send = (response, status, body) => {
const payload = JSON.stringify(body); const payload = JSON.stringify(body);
response.writeHead(status, { "content-type": "application/json", "content-length": Buffer.byteLength(payload) }); response.writeHead(status, { "content-type": "application/json", "content-length": Buffer.byteLength(payload) });
@@ -1104,7 +1104,7 @@ task13_assert_server_runtime() {
status="$TASK13_TMP/server-auth-status.json" status="$TASK13_TMP/server-auth-status.json"
task13_run_logged "server static OIDC status" "$TASK13_THT" --installation "$TASK13_INSTALLATION" auth status --json task13_run_logged "server static OIDC status" "$TASK13_THT" --installation "$TASK13_INSTALLATION" auth status --json
"$TASK13_THT" --installation "$TASK13_INSTALLATION" auth status --json >"$status" "$TASK13_THT" --installation "$TASK13_INSTALLATION" auth status --json >"$status"
node -e 'const value=JSON.parse(require("fs").readFileSync(process.argv[1], "utf8")); if(value.mode!=="oidc"||typeof value.configRevision!=="string"||value.configRevision.length!==64) process.exit(1)' "$status" \ node -e 'const value=JSON.parse(require("fs").readFileSync(process.argv[1], "utf8")); if(value.mode!=="oidc"||!/^sha256:[0-9a-f]{64}$/.test(value.configRevision)) process.exit(1)' "$status" \
|| task13_fail "server static OIDC status was not valid" || task13_fail "server static OIDC status was not valid"
diagnostics="$TASK13_TMP/server-auth-diagnostics.json" diagnostics="$TASK13_TMP/server-auth-diagnostics.json"
task13_run_logged "server live OIDC diagnostics" "$TASK13_THT" --installation "$TASK13_INSTALLATION" auth check --json task13_run_logged "server live OIDC diagnostics" "$TASK13_THT" --installation "$TASK13_INSTALLATION" auth check --json
+24 -12
View File
@@ -395,6 +395,7 @@ type renderedCompose struct {
} `json:"volumes"` } `json:"volumes"`
Services map[string]struct { Services map[string]struct {
Image string `json:"image"` Image string `json:"image"`
ContainerName string `json:"container_name"`
} `json:"services"` } `json:"services"`
} }
@@ -474,18 +475,13 @@ func imageIdentities(ctx context.Context, installation config.Installation, runn
if err != nil { if err != nil {
return nil, err return nil, err
} }
matching := make([]composeImageIdentity, 0, len(inspected)) expectedContainer := rendered.Services[name].ContainerName
for _, item := range inspected { if expectedContainer == "" {
if item.Service == "" || item.Service == name { expectedContainer = installation.ProjectName() + "-" + name + "-1"
matching = append(matching, item)
} }
} id, err := selectComposeImageIdentity(name, expectedContainer, inspected)
if len(matching) > 1 { if err != nil {
return nil, errors.New("Docker Compose returned invalid image identities") return nil, err
}
id := ""
if len(matching) == 1 {
id = matching[0].ID
} }
images = append(images, ImageIdentity{Service: name, Reference: rendered.Services[name].Image, ID: id}) images = append(images, ImageIdentity{Service: name, Reference: rendered.Services[name].Image, ID: id})
} }
@@ -500,7 +496,7 @@ type composeImageIdentity struct {
func decodeComposeImageIdentities(value string) ([]composeImageIdentity, error) { func decodeComposeImageIdentities(value string) ([]composeImageIdentity, error) {
trimmed := strings.TrimSpace(value) trimmed := strings.TrimSpace(value)
if trimmed == "" { if trimmed == "" || trimmed == "null" {
return nil, nil return nil, nil
} }
var identities []composeImageIdentity var identities []composeImageIdentity
@@ -530,6 +526,22 @@ func decodeComposeImageIdentities(value string) ([]composeImageIdentity, error)
return identities, nil return identities, nil
} }
func selectComposeImageIdentity(service string, expectedContainer string, identities []composeImageIdentity) (string, error) {
if len(identities) == 0 {
return "", nil
}
matching := make([]composeImageIdentity, 0, len(identities))
for _, item := range identities {
if item.Service == service || item.Service == "" && item.ContainerName == expectedContainer {
matching = append(matching, item)
}
}
if len(matching) != 1 {
return "", errors.New("Docker Compose returned invalid image identities")
}
return matching[0].ID, nil
}
func installationRunning(ctx context.Context, installation config.Installation, runner archiveRunner) (bool, error) { func installationRunning(ctx context.Context, installation config.Installation, runner archiveRunner) (bool, error) {
result, err := runner.Run(ctx, installation.ComposeArgs("ps", "--all", "--format", "json"), nil) result, err := runner.Run(ctx, installation.ComposeArgs("ps", "--all", "--format", "json"), nil)
if err != nil { if err != nil {
+63 -2
View File
@@ -23,7 +23,7 @@ import (
var requiredTestVolumes = []string{"settings", "pi-state", "workspace-registry", "workspace-secrets", "sessions", "qdrant-data", "embedding-models"} var requiredTestVolumes = []string{"settings", "pi-state", "workspace-registry", "workspace-secrets", "sessions", "qdrant-data", "embedding-models"}
func TestDecodeComposeImageIdentitiesAcceptsArrayAndStreamingJSON(t *testing.T) { func TestDecodeComposeImageIdentitiesAcceptsNonEmptyArrayAndStreamingJSON(t *testing.T) {
for name, input := range map[string]string{ for name, input := range map[string]string{
"array": `[{"ContainerName":"project-core-1","ID":"sha256:core"},{"ContainerName":"project-frontend-1","ID":"sha256:frontend"}]`, "array": `[{"ContainerName":"project-core-1","ID":"sha256:core"},{"ContainerName":"project-frontend-1","ID":"sha256:frontend"}]`,
"streaming": "{\"Service\":\"core\",\"ID\":\"sha256:core\"}\n{\"Service\":\"frontend\",\"ID\":\"sha256:frontend\"}\n", "streaming": "{\"Service\":\"core\",\"ID\":\"sha256:core\"}\n{\"Service\":\"frontend\",\"ID\":\"sha256:frontend\"}\n",
@@ -41,11 +41,72 @@ func TestDecodeComposeImageIdentitiesAcceptsArrayAndStreamingJSON(t *testing.T)
} }
func TestDecodeComposeImageIdentitiesAcceptsNoContainerForProfiledService(t *testing.T) { func TestDecodeComposeImageIdentitiesAcceptsNoContainerForProfiledService(t *testing.T) {
for _, input := range []string{"", "[]"} { for name, input := range map[string]string{
"empty output": "",
"empty array": "[]",
"null": "null",
} {
t.Run(name, func(t *testing.T) {
identities, err := decodeComposeImageIdentities(input) identities, err := decodeComposeImageIdentities(input)
if err != nil || len(identities) != 0 { if err != nil || len(identities) != 0 {
t.Fatalf("decodeComposeImageIdentities(%q) = %#v, %v", input, identities, err) t.Fatalf("decodeComposeImageIdentities(%q) = %#v, %v", input, identities, err)
} }
})
}
}
func TestDecodeComposeImageIdentitiesRejectsMalformedOrIncompleteOutput(t *testing.T) {
for name, input := range map[string]string{
"malformed JSON": "[",
"scalar JSON": "true",
"empty object": "{}",
"null array element": "[null]",
"missing image ID": `[{"Service":"core"}]`,
"trailing document": "null\n{}",
"trailing malformed bytes": `null garbage`,
} {
t.Run(name, func(t *testing.T) {
if identities, err := decodeComposeImageIdentities(input); err == nil {
t.Fatalf("decodeComposeImageIdentities(%q) = %#v, nil; want error", input, identities)
}
})
}
}
func TestSelectComposeImageIdentityUsesExactComposeContainerWhenServiceIsAbsent(t *testing.T) {
identities := []composeImageIdentity{
{ContainerName: "project-core-1", ID: "sha256:core"},
{ContainerName: "project-llm", ID: "sha256:shared-image"},
}
id, err := selectComposeImageIdentity("core", "project-core-1", identities)
if err != nil {
t.Fatal(err)
}
if id != "sha256:core" {
t.Fatalf("selected image ID = %q, want sha256:core", id)
}
}
func TestSelectComposeImageIdentityDoesNotFailOpen(t *testing.T) {
for name, identities := range map[string][]composeImageIdentity{
"unrelated container only": {{ContainerName: "project-llm", ID: "sha256:shared-image"}},
"duplicate service": {
{Service: "core", ID: "sha256:first"},
{Service: "core", ID: "sha256:second"},
},
} {
t.Run(name, func(t *testing.T) {
if id, err := selectComposeImageIdentity("core", "project-core-1", identities); err == nil {
t.Fatalf("selectComposeImageIdentity() = %q, nil; want error", id)
}
})
}
}
func TestSelectComposeImageIdentityAcceptsSemanticallyEmptyOutput(t *testing.T) {
id, err := selectComposeImageIdentity("workspace-maintenance", "project-workspace-maintenance-1", nil)
if err != nil || id != "" {
t.Fatalf("selectComposeImageIdentity() = %q, %v; want empty identity", id, err)
} }
} }
+7
View File
@@ -572,6 +572,13 @@ func validateVolumeTar(ctx context.Context, member *zip.File) error {
return fmt.Errorf("read volume archive: %w", err) return fmt.Errorf("read volume archive: %w", err)
} }
name := strings.TrimSuffix(header.Name, "/") name := strings.TrimSuffix(header.Name, "/")
if name == "." {
if header.Typeflag != tar.TypeDir {
return errors.New("volume archive root marker is not a directory")
}
continue
}
name = strings.TrimPrefix(name, "./")
if _, err := validateArchiveMemberPath(name); err != nil { if _, err := validateArchiveMemberPath(name); err != nil {
return fmt.Errorf("volume archive path is unsafe: %w", err) return fmt.Errorf("volume archive path is unsafe: %w", err)
} }
@@ -233,6 +233,60 @@ func TestPreflightRejectsTraversalAndSymlinkInsideVolumeTar(t *testing.T) {
} }
} }
func TestPreflightAcceptsCanonicalTarRootDirectoryAndRelativeMembers(t *testing.T) {
installation := preflightTestInstallation(t)
var payload strings.Builder
writer := tar.NewWriter(&stringWriter{value: &payload})
for _, header := range []tar.Header{
{Name: "./", Mode: 0o755, Typeflag: tar.TypeDir},
{Name: "./payload", Mode: 0o600, Size: 1, Typeflag: tar.TypeReg},
} {
if err := writer.WriteHeader(&header); err != nil {
t.Fatal(err)
}
if header.Size > 0 {
if _, err := writer.Write([]byte("x")); err != nil {
t.Fatal(err)
}
}
}
if err := writer.Close(); err != nil {
t.Fatal(err)
}
archive := filepath.Join(t.TempDir(), "canonical-volume.zip")
writePreflightArchive(t, archive, preflightArchiveSpec{entries: []preflightArchiveEntry{{
path: "volumes/sessions.tar", body: []byte(payload.String()), kind: EntryVolume,
}}})
result, err := Preflight(context.Background(), installation, PreflightRequest{Archive: archive, Confirm: true}, permissivePreflightDependencies())
if err != nil {
t.Fatal(err)
}
result.CloseArchive()
}
func TestPreflightRejectsNonDirectoryTarRootMarker(t *testing.T) {
installation := preflightTestInstallation(t)
var payload strings.Builder
writer := tar.NewWriter(&stringWriter{value: &payload})
header := tar.Header{Name: ".", Mode: 0o600, Size: 1, Typeflag: tar.TypeReg}
if err := writer.WriteHeader(&header); err != nil {
t.Fatal(err)
}
if _, err := writer.Write([]byte("x")); err != nil {
t.Fatal(err)
}
if err := writer.Close(); err != nil {
t.Fatal(err)
}
archive := filepath.Join(t.TempDir(), "unsafe-root-volume.zip")
writePreflightArchive(t, archive, preflightArchiveSpec{entries: []preflightArchiveEntry{{
path: "volumes/sessions.tar", body: []byte(payload.String()), kind: EntryVolume,
}}})
if _, err := Preflight(context.Background(), installation, PreflightRequest{Archive: archive, Confirm: true}, permissivePreflightDependencies()); err == nil {
t.Fatal("Preflight accepted a non-directory TAR root marker")
}
}
func TestPreflightRejectsArchivesThatExceedConfiguredProcessingLimits(t *testing.T) { func TestPreflightRejectsArchivesThatExceedConfiguredProcessingLimits(t *testing.T) {
installation := preflightTestInstallation(t) installation := preflightTestInstallation(t)
tests := []struct { tests := []struct {
+1 -1
View File
@@ -285,7 +285,7 @@ func safeRestoreParent(target string) bool {
func volumeRestoreCommand(volume string) []string { func volumeRestoreCommand(volume string) []string {
return []string{ return []string{
"run", "--rm", "--network", "none", "--mount", "type=volume,src=" + volume + ",dst=/target", "run", "--rm", "--interactive", "--network", "none", "--mount", "type=volume,src=" + volume + ",dst=/target",
helperImage, "sh", "-ceu", helperImage, "sh", "-ceu",
"rm -rf -- /target/* /target/.[!.]* /target/..?*; tar --numeric-owner -C /target -xf -", "rm -rf -- /target/* /target/.[!.]* /target/..?*; tar --numeric-owner -C /target -xf -",
} }
+13
View File
@@ -30,6 +30,19 @@ func TestRestorePublicPathUsesConcreteProductionPreflight(t *testing.T) {
} }
} }
func TestVolumeRestoreCommandKeepsTarInputOpenWithoutTTY(t *testing.T) {
args := volumeRestoreCommand("project_sessions")
wantPrefix := []string{"run", "--rm", "--interactive", "--network", "none"}
if len(args) < len(wantPrefix) || !equalStrings(args[:len(wantPrefix)], wantPrefix) {
t.Fatalf("volumeRestoreCommand() prefix = %q, want %q", args, wantPrefix)
}
for _, arg := range args {
if arg == "--tty" || arg == "-t" {
t.Fatalf("volumeRestoreCommand() requests a TTY: %q", args)
}
}
}
func TestRestoreRestoresVerifiedVolumesInManifestOrderBeforeAuthenticationReset(t *testing.T) { func TestRestoreRestoresVerifiedVolumesInManifestOrderBeforeAuthenticationReset(t *testing.T) {
installation := preflightTestInstallation(t) installation := preflightTestInstallation(t)
archive := filepath.Join(t.TempDir(), "restore-volumes.zip") archive := filepath.Join(t.TempDir(), "restore-volumes.zip")
+5 -2
View File
@@ -477,7 +477,8 @@ func setMaintenance(ctx context.Context, runner Runner, enabled bool) error {
if enabled { if enabled {
path = "activate" path = "activate"
} }
args := []string{"exec", "-T", "core", "curl", "-fsS", "-X", "POST", "http://127.0.0.1:8787/internal/maintenance/" + path} args := append([]string{"exec", "-T", "core", "curl", "-fsS"}, internalIdentityHeaders...)
args = append(args, "-X", "POST", "http://127.0.0.1:8787/internal/maintenance/"+path)
result, err := runCompose(ctx, runner, args...) result, err := runCompose(ctx, runner, args...)
status, valid := parseMaintenanceStatus(result.Stdout) status, valid := parseMaintenanceStatus(result.Stdout)
if err == nil && valid && status.Active == enabled && status.Admissions == 0 && !status.RecoveryRequired { if err == nil && valid && status.Active == enabled && status.Admissions == 0 && !status.RecoveryRequired {
@@ -517,7 +518,9 @@ func parseMaintenanceStatus(value string) (MaintenanceState, bool) {
} }
func MaintenanceStatus(ctx context.Context, runner Runner) (MaintenanceState, error) { func MaintenanceStatus(ctx context.Context, runner Runner) (MaintenanceState, error) {
result, err := runCompose(ctx, runner, "exec", "-T", "core", "curl", "-fsS", "http://127.0.0.1:8787/internal/maintenance/status") args := append([]string{"exec", "-T", "core", "curl", "-fsS"}, internalIdentityHeaders...)
args = append(args, "http://127.0.0.1:8787/internal/maintenance/status")
result, err := runCompose(ctx, runner, args...)
if err != nil { if err != nil {
return MaintenanceState{}, commandError("maintenance status check", result, err) return MaintenanceState{}, commandError("maintenance status check", result, err)
} }
+33
View File
@@ -779,6 +779,39 @@ func TestUpdateRequiresConfirmationAndDrainsActiveSessions(t *testing.T) {
assertCalled(t, fake.calls, "/internal/maintenance/deactivate") assertCalled(t, fake.calls, "/internal/maintenance/deactivate")
} }
func TestMaintenanceControlUsesExactLoopbackOperatorIdentity(t *testing.T) {
fake := newFakeRunner()
if err := setMaintenance(context.Background(), fake, true); err != nil {
t.Fatal(err)
}
fake.maintenance = true
if _, err := MaintenanceStatus(context.Background(), fake); err != nil {
t.Fatal(err)
}
for _, path := range []string{"/internal/maintenance/activate", "/internal/maintenance/status"} {
found := false
for _, call := range fake.calls {
if !strings.Contains(call, path) {
continue
}
found = true
for _, header := range []string{
"x-thoth-principal-issuer: tht",
"x-thoth-principal-subject: tht-maintenance",
"x-thoth-principal-display-name: Tht maintenance",
"x-thoth-is-admin: 1",
} {
if !strings.Contains(call, header) {
t.Fatalf("maintenance call %q lacks %q", call, header)
}
}
}
if !found {
t.Fatalf("maintenance call %q was not made", path)
}
}
}
func TestRollbackRestoresInterruptedOrPreviouslyRecordedState(t *testing.T) { func TestRollbackRestoresInterruptedOrPreviouslyRecordedState(t *testing.T) {
fake := newFakeRunner() fake := newFakeRunner()
statePath := filepath.Join(t.TempDir(), "state.json") statePath := filepath.Join(t.TempDir(), "state.json")