fix(ci,hasura,docs): FIFO and symlink races in ci config, path-selection note, junk hasura files, nself-image Go spec (D-1173, D-1174, D-1175, D-0628, D-0296)

free/ci/internal/config/migrate.go

@@ -165,6 +165,9 @@ func Migrate(root string) (*Migration, error) {
 			return nil, untranslatable(rel, 0, "gateway_routes", "gateway route probes are not jobs; remove the field or move the probes into a job run argv")
 		}
 		dir := filepath.ToSlash(filepath.Dir(rel))
+		if dir != "." && len(lf.Stages) > 0 {
+			m.Notes = append(m.Notes, rel+": the legacy runner ran every stage on every run; the migrated jobs get when.paths "+dir+"/** and run only when a file under that directory changed, so remove the when block from a job that must always run")
+		}
 		for i, st := range lf.Stages {
 			field := fmt.Sprintf("stages[%d]", i)
 			id := jobID(lf.Plugin, st.Name, taken)

free/ci/internal/config/migrate_render.go

@@ -99,6 +99,10 @@ func (m *Migration) Diff(root string) (string, error) {
 	return b.String(), nil
 }
 
+// testHookBeforeCreate runs between the destination check and the create;
+// tests use it to swap a path, as a concurrent local writer could.
+var testHookBeforeCreate = func(string) {}
+
 // Write creates root/.nself/ci.yaml. It refuses to replace a file that holds
 // other content (merge by hand) and succeeds quietly when the file is identical.
 func (m *Migration) Write(root string) error {
@@ -114,10 +118,18 @@ func (m *Migration) Write(root string) error {
 	} else if !os.IsNotExist(err) {
 		return &Error{Code: CodeInvalid, Problems: []Problem{{File: FileName, Message: err.Error()}}}
 	}
-	if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
+	testHookBeforeCreate(root)
+	// checkDestination looked at the path; a symlink swapped in since then must
+	// not carry the write out of the repository. os.Root confines every step.
+	r, err := os.OpenRoot(root)
+	if err != nil {
+		return err
+	}
+	defer func() { _ = r.Close() }()
+	if err := r.MkdirAll(filepath.ToSlash(filepath.Dir(FileName)), 0o755); err != nil {
 		return err
 	}
-	f, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o644)
+	f, err := r.OpenFile(filepath.ToSlash(FileName), os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o644)
 	if err != nil {
 		return err
 	}

free/ci/internal/config/migrate_test.go

@@ -7,6 +7,8 @@ import (
 	"path/filepath"
 	"strings"
 	"testing"
+
+	"github.com/nself-org/plugins/free/ci/internal/legacy"
 )
 
 // copyFixture copies testdata/<dir>/input into a temp repo root, turning each
@@ -334,3 +336,92 @@ func TestOracleSkipsManifestsWithoutAPluginKey(t *testing.T) {
 		}
 	}
 }
+
+// A .nself directory swapped for a symlink after checkDestination must not
+// carry the write out of the repository.
+func TestWriteRefusesSymlinkSwappedAfterCheck(t *testing.T) {
+	root := copyFixture(t, filepath.Join("legacy", "gauth"))
+	outside := t.TempDir()
+	m, err := Migrate(root)
+	if err != nil {
+		t.Fatal(err)
+	}
+	testHookBeforeCreate = func(r string) {
+		testHookBeforeCreate = func(string) {}
+		if err := os.Symlink(outside, filepath.Join(r, ".nself")); err != nil {
+			t.Error(err)
+		}
+	}
+	t.Cleanup(func() { testHookBeforeCreate = func(string) {} })
+	if err := m.Write(root); err == nil {
+		t.Fatal("wrote through a .nself symlink swapped in after the check")
+	}
+	if entries, _ := os.ReadDir(outside); len(entries) != 0 {
+		t.Fatalf("file created outside the repo: %v", entries)
+	}
+}
+
+// A manifest below the root migrates to path-gated jobs the legacy runner
+// never had; the migration must say so.
+func TestMigrateNotesPathSelection(t *testing.T) {
+	m, err := Migrate(copyFixture(t, filepath.Join("legacy", "multi")))
+	if err != nil {
+		t.Fatal(err)
+	}
+	var got []string
+	for _, n := range m.Notes {
+		if strings.Contains(n, "when.paths") {
+			got = append(got, n)
+		}
+	}
+	if len(got) != 1 || !strings.HasPrefix(got[0], "free/other/.ci.yaml: ") || !strings.Contains(got[0], "ran every stage on every run") {
+		t.Errorf("path-selection note: %v (all notes %v)", got, m.Notes)
+	}
+}
+
+// The path-selection note says the legacy runner ran every stage on every run,
+// in the manifest's own directory. Run the real legacy runner twice over the
+// same unchanged tree and check both halves of that claim.
+func TestLegacyRunnerRanEveryStageEveryRun(t *testing.T) {
+	root := t.TempDir()
+	dir := filepath.Join(root, "free", "other")
+	if err := os.MkdirAll(dir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	// Block-list cmd: the legacy line scanner does not read flow lists.
+	manifest := "plugin: other\nstages:\n" +
+		"  - name: first\n    cmd:\n    - sh\n    - -c\n    - echo x >> first.log\n" +
+		"  - name: second\n    cmd:\n    - sh\n    - -c\n    - echo x >> second.log\n"
+	if err := os.WriteFile(filepath.Join(dir, ".ci.yaml"), []byte(manifest), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	for run := 1; run <= 2; run++ {
+		results, err := legacy.DiscoverAndRunPipeline(root, "", 30, false)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if len(results) != 2 {
+			t.Fatalf("run %d: %d results, want one per stage", run, len(results))
+		}
+		for _, log := range []string{"first.log", "second.log"} {
+			data, err := os.ReadFile(filepath.Join(dir, log))
+			if err != nil {
+				t.Fatalf("run %d: %s not written in the manifest directory: %v", run, log, err)
+			}
+			if got := strings.Count(string(data), "x"); got != run {
+				t.Errorf("run %d: %s has %d lines; the stage must run once per run with no change filter", run, log, got)
+			}
+		}
+	}
+	m, err := Migrate(root)
+	if err != nil {
+		t.Fatal(err)
+	}
+	found := false
+	for _, n := range m.Notes {
+		found = found || strings.Contains(n, "ran every stage on every run")
+	}
+	if !found {
+		t.Errorf("the migration dropped the every-run note: %v", m.Notes)
+	}
+}

free/ci/internal/config/readfile.go

@@ -6,7 +6,9 @@ package config
 // file or exceeds MaxFileBytes.
 // Constraints: a symlink is followed only to a regular file; a device (a link
 // to /dev/zero) or a FIFO is refused before it is opened, so a read can
-// neither fill memory nor block forever.
+// neither fill memory nor block forever. The path check is only an early
+// exit: the file is opened without blocking and its type is checked again on
+// the open descriptor, so a swap between the check and the open cannot hang.
 
 import (
 	"fmt"
@@ -17,6 +19,10 @@ import (
 // MaxFileBytes bounds a config or legacy manifest.
 const MaxFileBytes = 1 << 20
 
+// testHookBeforeOpen runs between the path check and the open; tests use it
+// to swap the file, as a concurrent local writer could.
+var testHookBeforeOpen = func(string) {}
+
 func readRegular(path string) ([]byte, error) {
 	info, err := os.Lstat(path)
 	if err != nil {
@@ -33,11 +39,17 @@ func readRegular(path string) ([]byte, error) {
 	if !info.Mode().IsRegular() {
 		return nil, fmt.Errorf("%s is not a regular file", path)
 	}
-	f, err := os.Open(path)
+	testHookBeforeOpen(path)
+	f, err := openNonBlocking(path)
 	if err != nil {
 		return nil, err
 	}
 	defer func() { _ = f.Close() }()
+	if fi, err := f.Stat(); err != nil {
+		return nil, err
+	} else if !fi.Mode().IsRegular() {
+		return nil, fmt.Errorf("%s is not a regular file", path)
+	}
 	data, err := io.ReadAll(io.LimitReader(f, MaxFileBytes+1))
 	if err != nil {
 		return nil, err

free/ci/internal/config/readfile_other.go

@@ -0,0 +1,8 @@
+//go:build !unix
+
+package config
+
+import "os"
+
+// openNonBlocking opens path read-only; this platform has no FIFOs to block on.
+func openNonBlocking(path string) (*os.File, error) { return os.Open(path) }

free/ci/internal/config/readfile_unix.go

@@ -0,0 +1,14 @@
+//go:build unix
+
+package config
+
+import (
+	"os"
+	"syscall"
+)
+
+// openNonBlocking opens path read-only without blocking on a FIFO and without
+// taking a terminal as the controlling one.
+func openNonBlocking(path string) (*os.File, error) {
+	return os.OpenFile(path, os.O_RDONLY|syscall.O_NONBLOCK|syscall.O_NOCTTY, 0)
+}

free/ci/internal/config/readfile_unix_test.go

@@ -52,3 +52,32 @@ func TestMigrateDestinationFIFO(t *testing.T) {
 		t.Fatal("Diff blocked on a FIFO")
 	}
 }
+
+// A regular file swapped for a FIFO after the path check must be refused on
+// the open descriptor, not hang the open.
+func TestReadRegularFIFOSwappedAfterCheck(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "ci.yaml")
+	if err := os.WriteFile(path, []byte("version: 1\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	testHookBeforeOpen = func(p string) {
+		testHookBeforeOpen = func(string) {}
+		if err := os.Remove(p); err != nil {
+			t.Error(err)
+		}
+		if err := syscall.Mkfifo(p, 0o644); err != nil {
+			t.Error(err)
+		}
+	}
+	t.Cleanup(func() { testHookBeforeOpen = func(string) {} })
+	done := make(chan error, 1)
+	go func() { _, err := readRegular(path); done <- err }()
+	select {
+	case err := <-done:
+		if err == nil {
+			t.Fatal("FIFO swapped in after the check was read")
+		}
+	case <-time.After(5 * time.Second):
+		t.Fatal("readRegular blocked on a FIFO swapped in after the check")
+	}
+}

free/media-processing/hasura/metadata/databases/default/tables/np_mediap_encoding_profiles np_mediap_jobs np_mediap_job_outputs np_mediap_hls_manifests np_mediap_subtitles np_mediap_trickplay np_mediap_webhook_events np_mediap_watcher_events np_mediap_uploads.yaml → free/media-processing/hasura/metadata/databases/default/tables/np_mediap_encoding_profiles.yaml

@@ -1,14 +1,6 @@
 - table:
     schema: public
     name: np_mediap_encoding_profiles
-np_mediap_jobs
-np_mediap_job_outputs
-np_mediap_hls_manifests
-np_mediap_subtitles
-np_mediap_trickplay
-np_mediap_webhook_events
-np_mediap_watcher_events
-np_mediap_uploads
   select_permissions:
     - role: user
       permission:

free/media-processing/hasura/metadata/databases/default/tables/np_mediap_hls_manifests.yaml

@@ -0,0 +1,36 @@
+- table:
+    schema: public
+    name: np_mediap_hls_manifests
+  select_permissions:
+    - role: user
+      permission:
+        columns: '*'
+        filter:
+          source_account_id:
+            _eq: X-Hasura-Source-Account-Id
+        allow_aggregations: true
+    - role: nself_admin
+      permission:
+        columns: '*'
+        filter: {}
+        allow_aggregations: true
+  insert_permissions:
+    - role: user
+      permission:
+        columns: '*'
+        check:
+          source_account_id:
+            _eq: X-Hasura-Source-Account-Id
+    - role: nself_admin
+      permission:
+        columns: '*'
+        check: {}
+  delete_permissions:
+    - role: user
+      permission:
+        filter:
+          source_account_id:
+            _eq: X-Hasura-Source-Account-Id
+    - role: nself_admin
+      permission:
+        filter: {}

free/media-processing/hasura/metadata/databases/default/tables/np_mediap_job_outputs.yaml

@@ -0,0 +1,36 @@
+- table:
+    schema: public
+    name: np_mediap_job_outputs
+  select_permissions:
+    - role: user
+      permission:
+        columns: '*'
+        filter:
+          source_account_id:
+            _eq: X-Hasura-Source-Account-Id
+        allow_aggregations: true
+    - role: nself_admin
+      permission:
+        columns: '*'
+        filter: {}
+        allow_aggregations: true
+  insert_permissions:
+    - role: user
+      permission:
+        columns: '*'
+        check:
+          source_account_id:
+            _eq: X-Hasura-Source-Account-Id
+    - role: nself_admin
+      permission:
+        columns: '*'
+        check: {}
+  delete_permissions:
+    - role: user
+      permission:
+        filter:
+          source_account_id:
+            _eq: X-Hasura-Source-Account-Id
+    - role: nself_admin
+      permission:
+        filter: {}

free/media-processing/hasura/metadata/databases/default/tables/np_mediap_jobs.yaml

@@ -0,0 +1,36 @@
+- table:
+    schema: public
+    name: np_mediap_jobs
+  select_permissions:
+    - role: user
+      permission:
+        columns: '*'
+        filter:
+          source_account_id:
+            _eq: X-Hasura-Source-Account-Id
+        allow_aggregations: true
+    - role: nself_admin
+      permission:
+        columns: '*'
+        filter: {}
+        allow_aggregations: true
+  insert_permissions:
+    - role: user
+      permission:
+        columns: '*'
+        check:
+          source_account_id:
+            _eq: X-Hasura-Source-Account-Id
+    - role: nself_admin
+      permission:
+        columns: '*'
+        check: {}
+  delete_permissions:
+    - role: user
+      permission:
+        filter:
+          source_account_id:
+            _eq: X-Hasura-Source-Account-Id
+    - role: nself_admin
+      permission:
+        filter: {}