Skip to content

Commit a8b5bed

Browse files
fix(policies): normalize downloaded assets to Unix format (#1427)
Scripts and AppArmor profiles are downloaded from the Windows AD server, where administrators author them with Windows tooling. Those editors routinely save files with CRLF line endings and a leading UTF-8 BOM. On Linux both are silent foot-guns: a CRLF shebang makes the kernel look for an interpreter path with a trailing carriage return, and a BOM sits in front of the shebang entirely, so the script fails to execute even though it looks correct. The same byte sequences trip up AppArmor profile parsing. Administrators should not have to remember to re-save every asset in Unix format for it to work. Sanitizing at the asset compression funnel cleans every asset exactly once per refresh and transparently covers all current and future asset consumers (scripts, apparmor) from a single place, since they all read from the compressed database. Files that look binary (they contain a NUL byte) are left byte-for-byte intact so non-text assets are never corrupted. Closes #1425 UDENG-10844
2 parents dcb58f4 + 177d9e2 commit a8b5bed

3 files changed

Lines changed: 219 additions & 2 deletions

File tree

internal/policies/export_test.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,11 @@ const (
99
PoliciesFileName = policiesFileName
1010
)
1111

12+
// SanitizeAssetContent exposes sanitizeAssetContent for testing.
13+
func SanitizeAssetContent(content []byte) []byte {
14+
return sanitizeAssetContent(content)
15+
}
16+
1217
// WithGDM specifies a personalized gdm manager.
1318
func WithGDM(m *gdm.Manager) Option {
1419
return func(o *options) error {

internal/policies/policies.go

Lines changed: 80 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package policies
66

77
import (
88
"archive/zip"
9+
"bytes"
910
"context"
1011
"errors"
1112
"io"
@@ -347,13 +348,37 @@ func CompressAssets(ctx context.Context, p string) (err error) {
347348
}
348349

349350
// #nosec G122 -- This is a path controlled by us
350-
// Copy file content
351+
// Assets are authored on and downloaded from the Windows AD server, so
352+
// text files routinely carry CRLF line endings or a leading UTF-8 BOM
353+
// that would otherwise break script execution and policy parsing on
354+
// Linux. Sanitize text assets, but binary assets (which can be large)
355+
// are stream-copied unchanged to avoid buffering them entirely.
351356
srcF, err := os.Open(path)
352357
if err != nil {
353358
return err
354359
}
355360
defer srcF.Close()
356-
if _, err = io.Copy(fZip, srcF); err != nil {
361+
362+
binary, err := isBinary(srcF)
363+
if err != nil {
364+
return err
365+
}
366+
if _, err := srcF.Seek(0, io.SeekStart); err != nil {
367+
return err
368+
}
369+
370+
if binary {
371+
if _, err := io.Copy(fZip, srcF); err != nil {
372+
return err
373+
}
374+
return nil
375+
}
376+
377+
content, err := io.ReadAll(srcF)
378+
if err != nil {
379+
return err
380+
}
381+
if _, err = fZip.Write(sanitizeAssetContent(content)); err != nil {
357382
return err
358383
}
359384

@@ -363,6 +388,59 @@ func CompressAssets(ctx context.Context, p string) (err error) {
363388
return err
364389
}
365390

391+
// utf8BOM is the UTF-8 byte order mark that Windows editors (e.g. Notepad,
392+
// PowerShell ISE) frequently prepend to files. When present at the start of a
393+
// script it sits before the shebang and prevents the kernel from recognizing
394+
// the interpreter.
395+
var utf8BOM = []byte{0xEF, 0xBB, 0xBF}
396+
397+
// isBinary streams through r and reports whether it contains a NUL byte, the
398+
// conventional signal that content is binary and must be preserved
399+
// byte-for-byte. It reads in chunks so large binary assets are not buffered in
400+
// memory entirely.
401+
func isBinary(r io.Reader) (bool, error) {
402+
buf := make([]byte, 32*1024)
403+
for {
404+
n, err := r.Read(buf)
405+
if bytes.IndexByte(buf[:n], 0) != -1 {
406+
return true, nil
407+
}
408+
if err == io.EOF {
409+
return false, nil
410+
}
411+
if err != nil {
412+
return false, err
413+
}
414+
}
415+
}
416+
417+
// sanitizeAssetContent normalizes Windows-specific byte sequences in a
418+
// downloaded asset so it works out of the box on Linux:
419+
// - a leading UTF-8 BOM is removed;
420+
// - CRLF (\r\n) line endings are converted to LF (\n).
421+
//
422+
// Assets are authored on and downloaded from the Windows AD server, where these
423+
// sequences are routinely introduced by editors. They break script execution
424+
// (a CRLF shebang resolves to an interpreter path with a trailing \r) and
425+
// policy parsing, forcing administrators to remember to save assets in Unix
426+
// format.
427+
//
428+
// Files that look binary (i.e. contain a NUL byte) are returned unchanged to
429+
// avoid corrupting non-text assets an administrator may have placed alongside
430+
// scripts and profiles.
431+
func sanitizeAssetContent(content []byte) []byte {
432+
// A NUL byte is the conventional signal that content is binary and must be
433+
// preserved byte-for-byte.
434+
if bytes.IndexByte(content, 0) != -1 {
435+
return content
436+
}
437+
438+
content = bytes.TrimPrefix(content, utf8BOM)
439+
content = bytes.ReplaceAll(content, []byte("\r\n"), []byte("\n"))
440+
441+
return content
442+
}
443+
366444
// GetUniqueRules return order rules, with one entry per key for a given type.
367445
// Returned file is a map of type to its entries.
368446
func (pols Policies) GetUniqueRules() map[string][]entry.Entry {

internal/policies/policies_test.go

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -518,6 +518,140 @@ func TestCompressAssets(t *testing.T) {
518518
}
519519
}
520520

521+
func TestSanitizeAssetContent(t *testing.T) {
522+
t.Parallel()
523+
524+
bom := []byte{0xEF, 0xBB, 0xBF}
525+
withBOM := func(b []byte) []byte {
526+
return append(append([]byte{}, bom...), b...)
527+
}
528+
529+
tests := map[string]struct {
530+
content []byte
531+
532+
want []byte
533+
}{
534+
// Normalized cases
535+
"Plain LF content is left untouched": {
536+
content: []byte("#!/bin/sh\necho hello\n"),
537+
want: []byte("#!/bin/sh\necho hello\n"),
538+
},
539+
"CRLF is converted to LF": {
540+
content: []byte("#!/bin/sh\r\necho hello\r\n"),
541+
want: []byte("#!/bin/sh\necho hello\n"),
542+
},
543+
"Multiple CRLF are all converted": {
544+
content: []byte("a\r\nb\r\nc\r\n"),
545+
want: []byte("a\nb\nc\n"),
546+
},
547+
"Leading BOM is stripped": {
548+
content: withBOM([]byte("#!/bin/sh\n")),
549+
want: []byte("#!/bin/sh\n"),
550+
},
551+
"Leading BOM and CRLF are both normalized": {
552+
content: withBOM([]byte("#!/bin/sh\r\necho hi\r\n")),
553+
want: []byte("#!/bin/sh\necho hi\n"),
554+
},
555+
"Content consisting only of a BOM is emptied": {
556+
content: withBOM(nil),
557+
want: nil,
558+
},
559+
"Empty content is left untouched": {
560+
content: []byte{},
561+
want: nil,
562+
},
563+
564+
// Preservation cases
565+
"Lone CR is preserved": {
566+
content: []byte("progress\rdone\r\n"),
567+
want: []byte("progress\rdone\n"),
568+
},
569+
"BOM not at the start is preserved": {
570+
content: append([]byte("prefix"), withBOM([]byte("\n"))...),
571+
want: append([]byte("prefix"), withBOM([]byte("\n"))...),
572+
},
573+
574+
// Binary cases: a NUL byte means the file must be preserved verbatim.
575+
"Binary content with CRLF is preserved": {
576+
content: []byte("\x00\x01\x02\r\n"),
577+
want: []byte("\x00\x01\x02\r\n"),
578+
},
579+
"Binary content with a leading BOM is preserved": {
580+
content: withBOM([]byte("\x00binary\r\n")),
581+
want: withBOM([]byte("\x00binary\r\n")),
582+
},
583+
}
584+
585+
for name, tc := range tests {
586+
t.Run(name, func(t *testing.T) {
587+
t.Parallel()
588+
589+
got := policies.SanitizeAssetContent(tc.content)
590+
591+
// Compare as strings to gracefully handle nil vs empty slices.
592+
require.Equal(t, string(tc.want), string(got), "SanitizeAssetContent returned unexpected content")
593+
})
594+
}
595+
}
596+
597+
func TestCompressAssetsSanitizesContent(t *testing.T) {
598+
t.Parallel()
599+
600+
bom := []byte{0xEF, 0xBB, 0xBF}
601+
withBOM := func(b []byte) []byte {
602+
return append(append([]byte{}, bom...), b...)
603+
}
604+
605+
crlfScript := []byte("#!/bin/sh\r\necho crlf\r\n")
606+
wantCrlfScript := []byte("#!/bin/sh\necho crlf\n")
607+
bomScript := withBOM([]byte("#!/bin/sh\r\necho bom\r\n"))
608+
wantBomScript := []byte("#!/bin/sh\necho bom\n")
609+
lfScript := []byte("#!/bin/sh\necho lf\n")
610+
// A binary asset carrying both a BOM and CRLF must survive untouched.
611+
binaryAsset := withBOM([]byte("\x00\x01\r\n\x02"))
612+
613+
// content authored on Windows, mapped to the expected sanitized result.
614+
assets := map[string]struct {
615+
content []byte
616+
want []byte
617+
}{
618+
filepath.Join("scripts", "crlf.sh"): {content: crlfScript, want: wantCrlfScript},
619+
filepath.Join("scripts", "bom.sh"): {content: bomScript, want: wantBomScript},
620+
filepath.Join("scripts", "lf.sh"): {content: lfScript, want: lfScript},
621+
filepath.Join("scripts", "binary"): {content: binaryAsset, want: binaryAsset},
622+
filepath.Join("apparmor", "usr.bin"): {content: bomScript, want: wantBomScript},
623+
}
624+
625+
// Lay out the assets directory the same way it exists after a download.
626+
parentDir := t.TempDir()
627+
assetsDir := filepath.Join(parentDir, "assets")
628+
for relPath, asset := range assets {
629+
dest := filepath.Join(assetsDir, relPath)
630+
require.NoError(t, os.MkdirAll(filepath.Dir(dest), 0700), "Setup: can’t create asset subdirectory")
631+
require.NoError(t, os.WriteFile(dest, asset.content, 0600), "Setup: can’t write asset file")
632+
}
633+
634+
require.NoError(t, policies.CompressAssets(context.Background(), assetsDir), "CompressAssets should return no error but got one")
635+
636+
// Drop the uncompressed assets so the policies are loaded purely from the db,
637+
// then extract them again to inspect what was actually stored.
638+
require.NoError(t, os.RemoveAll(assetsDir), "Teardown: can’t remove uncompressed assets directory")
639+
require.NoError(t, os.WriteFile(filepath.Join(parentDir, policies.PoliciesFileName), nil, 0600), "Setup: can’t create empty policy cache file")
640+
641+
pols, err := policies.NewFromCache(context.Background(), parentDir)
642+
require.NoError(t, err, "NewFromCache should return no error but got one")
643+
defer pols.Close()
644+
645+
extractDir := filepath.Join(t.TempDir(), "extracted")
646+
require.NoError(t, pols.SaveAssetsTo(context.Background(), ".", extractDir, -1, -1), "SaveAssetsTo should return no error but got one")
647+
648+
for relPath, asset := range assets {
649+
got, err := os.ReadFile(filepath.Join(extractDir, relPath))
650+
require.NoError(t, err, "should be able to read extracted asset %q", relPath)
651+
require.Equal(t, string(asset.want), string(got), "unexpected stored content for asset %q", relPath)
652+
}
653+
}
654+
521655
func TestGetUniqueRules(t *testing.T) {
522656
t.Parallel()
523657

0 commit comments

Comments
 (0)