From 6d2822bdfc7ee0302c706d53273f28d7adc43404 Mon Sep 17 00:00:00 2001
From: James Maes <jmaes@dmdbrands.com>
Date: Thu, 4 Jun 2026 10:58:49 -0500
Subject: [PATCH] fix: preserve shebang universally so headers never precede #!

---
 CHANGELOG.md                       |  5 ++
 internal/applier/applier_test.go   | 45 ++++++++++++++++
 internal/detect/detect.go          |  8 ++-
 internal/detect/detect_test.go     | 53 +++++++++++++++++++
 internal/filetype/filetype.go      |  8 ++-
 internal/filetype/filetype_test.go |  9 ++--
 internal/render/render.go          | 11 ++++
 internal/render/render_test.go     | 83 ++++++++++++++++++++++++++++++
 8 files changed, 216 insertions(+), 6 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8f77f45..99daf89 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [0.2.1] - 2026-06-04
+
+### Fixed
+- Preserve a leading `#!` shebang universally (not per-type), so `apply` never inserts the header before the shebang. Fixes corruption of shebang-bearing scripts whose type lacked an explicit shebang rule (e.g. a PHP CLI script `#!/usr/bin/env php` + `<?php`), where the block-comment header was written at byte 0, breaking the file.
+
 ## [0.2.0] - 2026-06-03
 
 ### Added
diff --git a/internal/applier/applier_test.go b/internal/applier/applier_test.go
index fc1df09..4607f12 100644
--- a/internal/applier/applier_test.go
+++ b/internal/applier/applier_test.go
@@ -33,6 +33,23 @@ func goFileType() model.FileType {
 	}
 }
 
+// phpFileType returns a block-comment PHP file type whose preserve rules mirror the
+// builtin table: BOM, then a (universal) shebang, then the <?php open tag, all before
+// the header. It is defined locally for the same reason goFileType is -- to avoid
+// depending on the filetype package's table from the applier tests.
+func phpFileType() model.FileType {
+	return model.FileType{
+		Name:         "PHP",
+		Extensions:   []string{".php"},
+		CommentStyle: model.CommentStyle{Block: true, Open: "/*", Close: "*/"},
+		PreserveFirst: []model.PreserveRule{
+			{Kind: model.PreserveBOM, Before: false},
+			{Kind: model.PreserveShebang, Before: false},
+			{Kind: model.PreservePHPOpen, Before: false},
+		},
+	}
+}
+
 // agplConfig is the canonical apply config: AGPL, reuse+notice, an explicit year so
 // the rendered header is deterministic and independent of the wall clock.
 func agplConfig() model.Config {
@@ -370,6 +387,34 @@ func TestApplyFile(t *testing.T) {
 	})
 }
 
+// TestApplyFileShebangPHPIdempotent is the applier-level regression guard for the
+// universal-shebang fix on a BLOCK-comment type. A PHP CLI script begins with a "#!"
+// line then "<?php"; the header must land after both, and a second apply must be a
+// byte-identical no-op. Before the fix the header was spliced above the shebang, so the
+// second pass re-detected differently and the result was not idempotent.
+func TestApplyFileShebangPHPIdempotent(t *testing.T) {
+	content := []byte("#!/usr/bin/env php\n<?php\necho 'hi';\n")
+
+	once, _, action1, err := ApplyFile(content, phpFileType(), agplHeaderFunc(t))
+	require.NoError(t, err)
+	require.Equal(t, "insert", action1)
+
+	got := string(once)
+	assert.True(t, strings.HasPrefix(got, "#!/usr/bin/env php\n<?php\n"),
+		"the shebang stays line 1 and <?php line 2, with the header after both; got %q", got)
+	idxShebang := strings.Index(got, "#!")
+	idxHeader := strings.Index(got, "SPDX-License-Identifier: AGPL-3.0-or-later")
+	require.GreaterOrEqual(t, idxHeader, 0)
+	assert.Equal(t, 0, idxShebang, "the shebang must start at byte 0")
+	assert.Less(t, idxShebang, idxHeader, "the header is inserted after the shebang")
+
+	twice, diff2, action2, err := ApplyFile(once, phpFileType(), agplHeaderFunc(t))
+	require.NoError(t, err)
+	assert.Equal(t, "none", action2, "a second apply detects the managed header and changes nothing")
+	assert.Empty(t, diff2)
+	assert.Equal(t, once, twice, "apply twice must be byte-identical")
+}
+
 // --- ManageLicenseFiles ---------------------------------------------------
 
 // TestManageLicenseFilesUnknownLicense verifies an unknown SPDX id is rejected
diff --git a/internal/detect/detect.go b/internal/detect/detect.go
index ee1baf2..6fa1557 100644
--- a/internal/detect/detect.go
+++ b/internal/detect/detect.go
@@ -178,7 +178,13 @@ func PreserveBoundary(content []byte, ft model.FileType) (int, error) {
 		pos += len(utf8BOM)
 	}
 
-	allowShebang := hasRule(ft, model.PreserveShebang, false)
+	// A leading "#!" is a kernel-level shebang valid for ANY executable script
+	// regardless of language, so it is preserved universally rather than gated on the
+	// file type listing PreserveShebang. This stays safe because the line is only
+	// consumed when it actually starts with "#!" (which only a real shebang does) and
+	// only at the very top of the file (pos 0); it can never over-consume an ordinary
+	// hash comment further down. All OTHER per-type rules remain gated as before.
+	allowShebang := true
 	allowXML := hasRule(ft, model.PreserveXMLDecl, false)
 	allowPHP := hasRule(ft, model.PreservePHPOpen, false)
 	allowPragma := hasRule(ft, model.PreserveCodingPragma, false)
diff --git a/internal/detect/detect_test.go b/internal/detect/detect_test.go
index f2012ba..b5076fe 100644
--- a/internal/detect/detect_test.go
+++ b/internal/detect/detect_test.go
@@ -48,6 +48,35 @@ func hashFT() model.FileType {
 	}
 }
 
+// cFT is a block-comment type (C/C++ shape) that lists NO shebang rule -- only BOM.
+// It exists to prove the universal-shebang fix: PreserveBoundary must consume a leading
+// "#!" even though this type never lists PreserveShebang.
+func cFT() model.FileType {
+	return model.FileType{
+		Name:         "C",
+		Extensions:   []string{".c"},
+		CommentStyle: model.CommentStyle{Block: true, Open: "/*", Close: "*/"},
+		PreserveFirst: []model.PreserveRule{
+			{Kind: model.PreserveBOM, Before: false},
+		},
+	}
+}
+
+// phpFT is a block-comment type that preserves a (universal) shebang then the <?php
+// open tag, mirroring the builtin PHP table ordering.
+func phpFT() model.FileType {
+	return model.FileType{
+		Name:         "PHP",
+		Extensions:   []string{".php"},
+		CommentStyle: model.CommentStyle{Block: true, Open: "/*", Close: "*/"},
+		PreserveFirst: []model.PreserveRule{
+			{Kind: model.PreserveBOM, Before: false},
+			{Kind: model.PreserveShebang, Before: false},
+			{Kind: model.PreservePHPOpen, Before: false},
+		},
+	}
+}
+
 // xmlFT is a block-comment type that preserves an XML declaration and PHP open tag.
 func xmlFT() model.FileType {
 	return model.FileType{
@@ -273,6 +302,30 @@ func TestPreserveBoundary(t *testing.T) {
 			ft:       xmlFT(),
 			wantRest: "// body\n",
 		},
+		{
+			// Regression: a block-comment type that does NOT list PreserveShebang must
+			// still have its leading "#!" consumed, so the header lands below the shebang.
+			name:     "shebang consumed on block type without a shebang rule",
+			content:  "#!/usr/bin/tcc -run\nint main(void){return 0;}\n",
+			ft:       cFT(),
+			wantRest: "int main(void){return 0;}\n",
+		},
+		{
+			// Regression: PHP CLI script -- the universal shebang AND the <?php open tag
+			// are both consumed, in file order, so the header lands after line 2.
+			name:     "php shebang then php-open both consumed",
+			content:  "#!/usr/bin/env php\n<?php\necho 'hi';\n",
+			ft:       phpFT(),
+			wantRest: "echo 'hi';\n",
+		},
+		{
+			// The universal rule is inert without a shebang: a block-comment file whose
+			// first line is ordinary code keeps the boundary at zero.
+			name:     "no shebang leaves block-type boundary at zero",
+			content:  "int main(void){return 0;}\n",
+			ft:       cFT(),
+			wantRest: "int main(void){return 0;}\n",
+		},
 		{
 			name:     "package decl (Before rule) is never consumed",
 			content:  "package main\nfunc main(){}\n",
diff --git a/internal/filetype/filetype.go b/internal/filetype/filetype.go
index 1714aa3..7792635 100644
--- a/internal/filetype/filetype.go
+++ b/internal/filetype/filetype.go
@@ -166,12 +166,16 @@ var builtin = []model.FileType{
 		PreserveFirst: []model.PreserveRule{after(model.PreserveBOM), after(model.PreserveShebang)},
 	},
 
-	// PHP: header goes after the opening <?php tag.
+	// PHP: header goes after a CLI shebang (if any) and the opening <?php tag. The
+	// shebang is listed BEFORE php-open for table correctness/clarity; a "#!" line on
+	// a PHP CLI script is line 1, the <?php tag line 2, then the header. (The universal
+	// shebang preservation in render/detect already covers this; the explicit rule
+	// documents the ordering.)
 	{
 		Name:          "PHP",
 		Extensions:    []string{".php", ".phtml"},
 		CommentStyle:  block("/*", "*/"),
-		PreserveFirst: []model.PreserveRule{after(model.PreserveBOM), after(model.PreservePHPOpen)},
+		PreserveFirst: []model.PreserveRule{after(model.PreserveBOM), after(model.PreserveShebang), after(model.PreservePHPOpen)},
 	},
 
 	// Uncommentable formats: detected and reported, never written.
diff --git a/internal/filetype/filetype_test.go b/internal/filetype/filetype_test.go
index c35c36c..51754c8 100644
--- a/internal/filetype/filetype_test.go
+++ b/internal/filetype/filetype_test.go
@@ -91,11 +91,14 @@ func TestPreserveFirstOrdering(t *testing.T) {
 	require.Len(t, xml.PreserveFirst, 2)
 	assert.Equal(t, model.PreserveXMLDecl, xml.PreserveFirst[1].Kind)
 
-	// PHP: BOM then php-open.
+	// PHP: BOM, then shebang (CLI scripts), then php-open. The shebang precedes the
+	// <?php tag so a "#!" line stays line 1 on an executable PHP script.
 	php, ok := Lookup("index.php")
 	require.True(t, ok)
-	require.Len(t, php.PreserveFirst, 2)
-	assert.Equal(t, model.PreservePHPOpen, php.PreserveFirst[1].Kind)
+	require.Len(t, php.PreserveFirst, 3)
+	assert.Equal(t, model.PreserveBOM, php.PreserveFirst[0].Kind)
+	assert.Equal(t, model.PreserveShebang, php.PreserveFirst[1].Kind)
+	assert.Equal(t, model.PreservePHPOpen, php.PreserveFirst[2].Kind)
 
 	// Java: header goes BEFORE the package declaration.
 	java, ok := Lookup("Main.java")
diff --git a/internal/render/render.go b/internal/render/render.go
index 18f39e7..f1170e4 100644
--- a/internal/render/render.go
+++ b/internal/render/render.go
@@ -325,6 +325,17 @@ func preserveBoundary(content []byte, ft model.FileType) int {
 		pos += len(bom)
 	}
 
+	// A leading "#!" is a kernel-level shebang valid for ANY executable script
+	// regardless of language, so it is preserved universally rather than gated on the
+	// file type listing PreserveShebang. We consume it here (right after any BOM)
+	// independent of the rule list. This stays safe because the line is only consumed
+	// when it actually starts with "#!" (which only a real shebang does) and only at
+	// the very top of the file; it can never over-consume an ordinary hash comment
+	// below. All OTHER per-type rules below remain gated by ft.PreserveFirst as before.
+	pos = advanceLineIf(content, pos, func(line string) bool {
+		return strings.HasPrefix(line, "#!")
+	})
+
 	for _, rule := range ft.PreserveFirst {
 		if rule.Before {
 			// Header goes before this construct; do not advance past it.
diff --git a/internal/render/render_test.go b/internal/render/render_test.go
index a77b7e3..68e24a0 100644
--- a/internal/render/render_test.go
+++ b/internal/render/render_test.go
@@ -328,6 +328,89 @@ func TestSpliceInsertAfterPHPOpen(t *testing.T) {
 	assert.Less(t, idxOpen, idxHeader)
 }
 
+// TestSpliceShebangPreservedForBlockCommentTypes is the regression guard for the
+// universal-shebang fix. Before the fix, a leading "#!" was only preserved when the
+// file type explicitly listed PreserveShebang; a BLOCK-comment type either lacked the
+// rule (C/C++) or ordered it after php-open (PHP), so the header was spliced ABOVE the
+// shebang and the script stopped being executable. The fix preserves any leading "#!"
+// universally, so on every block-comment type the shebang must stay line 1 with the
+// header inserted strictly after it.
+func TestSpliceShebangPreservedForBlockCommentTypes(t *testing.T) {
+	cases := []struct {
+		name       string
+		sampleFile string
+		content    string
+		// wantPrefix is the exact byte prefix that must lead the output (the shebang,
+		// plus any prolog such as <?php, all before the header).
+		wantPrefix string
+	}{
+		{
+			// PHP CLI script: "#!" line 1, "<?php" line 2, header after both. PHP is a
+			// block-comment type whose rule list orders shebang before php-open.
+			name:       "php shebang then php-open",
+			sampleFile: "x.php",
+			content:    "#!/usr/bin/env php\n<?php\necho 'hi';\n",
+			wantPrefix: "#!/usr/bin/env php\n<?php\n",
+		},
+		{
+			// JavaScript is a block-comment type that DOES list PreserveShebang: a node
+			// CLI shebang must stay line 1 with the /* */ header after it.
+			name:       "javascript block-comment type with shebang",
+			sampleFile: "cli.js",
+			content:    "#!/usr/bin/env node\nconsole.log('hi');\n",
+			wantPrefix: "#!/usr/bin/env node\n",
+		},
+		{
+			// C/C++ is a block-comment type that does NOT list PreserveShebang at all.
+			// This is the case the universal rule exists for: preservation must NOT be
+			// gated on the type listing the rule. (binfmt_misc lets a "#!" launch any
+			// interpreter, so a shebang on a .c source is legal and must survive.)
+			name:       "c block-comment type without a shebang rule",
+			sampleFile: "x.c",
+			content:    "#!/usr/bin/tcc -run\nint main(void){return 0;}\n",
+			wantPrefix: "#!/usr/bin/tcc -run\n",
+		},
+	}
+
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			ft, header := makeHeaderLF(t, c.sampleFile)
+			out, action := Splice([]byte(c.content), ft, header, model.DetectedHeader{Present: false})
+			got := string(out)
+
+			assert.Equal(t, "insert", action)
+			assert.True(t, strings.HasPrefix(got, c.wantPrefix),
+				"the shebang (and any prolog) must remain the file's leading bytes; got %q", got)
+			idxShebang := strings.Index(got, "#!")
+			idxHeader := strings.Index(got, "SPDX-License-Identifier: MIT")
+			require.GreaterOrEqual(t, idxHeader, 0, "header must be present")
+			assert.Equal(t, 0, idxShebang, "the shebang must start at byte 0 (line 1)")
+			assert.Less(t, idxShebang, idxHeader, "the header must be inserted after the shebang")
+			assert.Contains(t, got, ft.CommentStyle.Open,
+				"the inserted header uses the type's block-comment open delimiter")
+		})
+	}
+}
+
+// TestSpliceNonShebangBlockCommentUnaffected proves the universal rule is inert when
+// there is no shebang: a block-comment file whose first line is ordinary code must get
+// the header at the very top (byte 0), exactly as before the fix. This pins the "only
+// consumed when the line actually starts with #!" safety claim.
+func TestSpliceNonShebangBlockCommentUnaffected(t *testing.T) {
+	ft, header := makeHeaderLF(t, "x.c")
+	content := []byte("int main(void){return 0;}\n")
+
+	out, action := Splice(content, ft, header, model.DetectedHeader{Present: false})
+	got := string(out)
+
+	assert.Equal(t, "insert", action)
+	assert.True(t, strings.HasPrefix(got, ft.CommentStyle.Open),
+		"with no shebang the header leads at byte 0; got %q", got)
+	idxHeader := strings.Index(got, "SPDX-License-Identifier: MIT")
+	idxBody := strings.Index(got, "int main")
+	assert.Less(t, idxHeader, idxBody, "header precedes the first real line when there is no shebang")
+}
+
 func TestSpliceInsertAfterCodingPragma(t *testing.T) {
 	ft, header := makeHeaderLF(t, "x.py")
 	content := []byte("#!/usr/bin/env python3\n# -*- coding: utf-8 -*-\nprint('hi')\n")
-- 
2.54.0