Repository: ysugimoto/falco
Pull request #108 (2022-09-28 14:07:01)

lint protected HTTP headers
Fastly protects some HTTP headers modification in VCL so the user could not modify them.
This PR validates the user will modify on `set`, `add`, `unset`, and `remove` statements and raise an error for that operation.

ref: https://developer.fastly.com/reference/http/http-headers/


Patch:
diff --git a/context/context.go b/context/context.go
index fda1fe8..f395fe7 100644
--- a/context/context.go
+++ b/context/context.go
@@ -194,22 +194,24 @@ func (c *Context) UserDefinedFunctionScope(name string, mode int, returnType typ
 // ```
 // declare local var.S STRING;
 // set var.S = "foo bar baz";
-// if (req.http.Host) {
-// 	if (var.S) {
-// 		if (var.S !~ "(foo)\s(bar)\s(baz)") { // make matched values first (1)
-// 			set req.http.First = "1";
-// 		}
-// 		set var.S = "hoge huga";
-// 		if (var.S ~ "(hoge)\s(huga)") { // override matched values (2)
-// 			set req.http.First = re.group.1;
-// 		}
-// 	}
-// 	set req.http.Third = re.group.2; // difficult to know which (1) or (2) matched result is used
-// }
 //
-// if (req.http.Host) {
-// 	set req.http.Fourth = re.group.3; // difficult to know which (1) or (2) matched result is used or empty
-// }
+//	if (req.http.Host) {
+//		if (var.S) {
+//			if (var.S !~ "(foo)\s(bar)\s(baz)") { // make matched values first (1)
+//				set req.http.First = "1";
+//			}
+//			set var.S = "hoge huga";
+//			if (var.S ~ "(hoge)\s(huga)") { // override matched values (2)
+//				set req.http.First = re.group.1;
+//			}
+//		}
+//		set req.http.Third = re.group.2; // difficult to know which (1) or (2) matched result is used
+//	}
+//
+//	if (req.http.Host) {
+//		set req.http.Fourth = re.group.3; // difficult to know which (1) or (2) matched result is used or empty
+//	}
+//
 // ```
 //
 // Therefore we will raise warning if matched value is overridden in subroutine.
diff --git a/linter/errors.go b/linter/errors.go
index f97c795..3195fcf 100644
--- a/linter/errors.go
+++ b/linter/errors.go
@@ -339,3 +339,11 @@ func DuplicatedUseForGotoDestination(m *ast.Meta, name string) *LintError {
 		Message:  fmt.Sprintf("goto destination %s has been already in use", name),
 	}
 }
+
+func ProtectedHTTPHeader(m *ast.Meta, name string) *LintError {
+	return &LintError{
+		Severity: ERROR,
+		Token:    m.Token,
+		Message:  fmt.Sprintf("%s HTTP header could not be modified in VCL", name),
+	}
+}
diff --git a/linter/helper.go b/linter/helper.go
index a86c742..fcb2753 100644
--- a/linter/helper.go
+++ b/linter/helper.go
@@ -466,3 +466,28 @@ func isTypeLiteral(node ast.Node) bool {
 		return false
 	}
 }
+
+// Some HTTP Headers is protected in Fastly.
+// @see https://developer.fastly.com/reference/http/http-headers/
+// Consider the character case, we always treat header names as lower-case.
+var protectedHeaderNames = map[string]struct{}{
+	"req.http.proxy-authenticate":  {},
+	"req.http.proxy-authorization": {},
+	"req.http.content-length":      {},
+	"req.http.content-range":       {},
+	"req.http.te":                  {},
+	"req.http.trailer":             {},
+	"req.http.expect":              {},
+	"req.http.transfer-encoding":   {},
+	"req.http.upgrade":             {},
+	"req.http.fastly-ff":           {},
+}
+
+// Check header name exists in protected header names
+func isProtectedHTTPHeaderName(name string) bool {
+	lower := strings.ToLower(name)
+	if _, ok := protectedHeaderNames[lower]; ok {
+		return true
+	}
+	return false
+}
diff --git a/linter/linter.go b/linter/linter.go
index c4d7018..8b07a89 100644
--- a/linter/linter.go
+++ b/linter/linter.go
@@ -837,6 +837,11 @@ func (l *Linter) lintSetStatement(stmt *ast.SetStatement, ctx *context.Context)
 		l.Error(InvalidName(stmt.Ident.GetMeta(), stmt.Ident.Value, "set").Match(SET_STATEMENT_SYNTAX))
 	}
 
+	// Check protected header will be modified
+	if isProtectedHTTPHeaderName(stmt.Ident.Value) {
+		l.Error(ProtectedHTTPHeader(stmt.Ident.GetMeta(), stmt.Ident.Value))
+	}
+
 	left, err := ctx.Set(stmt.Ident.Value)
 	if err != nil {
 		err := &LintError{
@@ -885,6 +890,11 @@ func (l *Linter) lintUnsetStatement(stmt *ast.UnsetStatement, ctx *context.Conte
 		l.Error(InvalidName(stmt.Ident.GetMeta(), stmt.Ident.Value, "unset").Match(UNSET_STATEMENT_SYNTAX))
 	}
 
+	// Check protected header will be modified
+	if isProtectedHTTPHeaderName(stmt.Ident.Value) {
+		l.Error(ProtectedHTTPHeader(stmt.Ident.GetMeta(), stmt.Ident.Value))
+	}
+
 	if err := ctx.Unset(stmt.Ident.Value); err != nil {
 		l.Error(&LintError{
 			Severity: ERROR,
@@ -901,6 +911,11 @@ func (l *Linter) lintRemoveStatement(stmt *ast.RemoveStatement, ctx *context.Con
 		l.Error(InvalidName(stmt.Ident.GetMeta(), stmt.Ident.Value, "remove").Match(REMOVE_STATEMENT_SYNTAX))
 	}
 
+	// Check protected header will be modified
+	if isProtectedHTTPHeaderName(stmt.Ident.Value) {
+		l.Error(ProtectedHTTPHeader(stmt.Ident.GetMeta(), stmt.Ident.Value))
+	}
+
 	if err := ctx.Unset(stmt.Ident.Value); err != nil {
 		l.Error(&LintError{
 			Severity: ERROR,
@@ -998,6 +1013,11 @@ func (l *Linter) lintAddStatement(stmt *ast.AddStatement, ctx *context.Context)
 		l.Error(InvalidName(stmt.Ident.GetMeta(), stmt.Ident.Value, "add").Match(ADD_STATEMENT_SYNTAX))
 	}
 
+	// Check protected header will be modified
+	if isProtectedHTTPHeaderName(stmt.Ident.Value) {
+		l.Error(ProtectedHTTPHeader(stmt.Ident.GetMeta(), stmt.Ident.Value))
+	}
+
 	// Add statement could use only for HTTP headers.
 	// https://developer.fastly.com/reference/vcl/statements/add/
 	if !strings.Contains(stmt.Ident.Value, "req.http.") &&


Test patch:
diff --git a/linter/linter_test.go b/linter/linter_test.go
index cab2d7d..41dfa90 100644
--- a/linter/linter_test.go
+++ b/linter/linter_test.go
@@ -2220,3 +2220,90 @@ func TestLintLogStatementr(t *testing.T) {
 	}
 
 }
+
+func TestLintProtectedHTTPHeaders(t *testing.T) {
+	tests := []struct {
+		name  string
+		value string
+	}{
+		{
+			name:  "Proxy-Authenticate",
+			value: "Basic realm=\"Access to the proxy site\"",
+		},
+		{
+			name:  "Proxy-Authorization",
+			value: "Basic foo",
+		},
+		{
+			name:  "Proxy-Authorization",
+			value: "Basic foo",
+		},
+		{
+			name:  "Content-Length",
+			value: "100",
+		},
+		{
+			name:  "Content-Range",
+			value: "bytes 200-100/12345",
+		},
+		{
+			name:  "TE",
+			value: "gzip",
+		},
+		{
+			name:  "Trailer",
+			value: "Expires",
+		},
+		{
+			name:  "Transfer-Encoding",
+			value: "gzip",
+		},
+		{
+			name:  "Expect",
+			value: "100-continue",
+		},
+		{
+			name:  "Upgrade",
+			value: "example/1",
+		},
+		{
+			name:  "Fastly-FF",
+			value: "qZarR/12OL0QOq4VyQPmqQ/CTp17AZv0d6cSG5nUSxU=!WDC!cache-wdc5548-WDC",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(fmt.Sprintf("%s in set statement", tt.name), func(t *testing.T) {
+			input := fmt.Sprintf(`
+	  sub vcl_recv {
+		  #FASTLY RECV
+		  set req.http.%s = "%s";
+	  }`, tt.name, tt.value)
+			assertError(t, input)
+		})
+		t.Run(fmt.Sprintf("%s in add statement", tt.name), func(t *testing.T) {
+			input := fmt.Sprintf(`
+	  sub vcl_recv {
+		  #FASTLY RECV
+		  add req.http.%s = "%s";
+	  }`, tt.name, tt.value)
+			assertError(t, input)
+		})
+		t.Run(fmt.Sprintf("%s in unset statement", tt.name), func(t *testing.T) {
+			input := fmt.Sprintf(`
+	  sub vcl_recv {
+		  #FASTLY RECV
+		  unset req.http.%s;
+	  }`, tt.name)
+			assertError(t, input)
+		})
+		t.Run(fmt.Sprintf("%s in remove statement", tt.name), func(t *testing.T) {
+			input := fmt.Sprintf(`
+	  sub vcl_recv {
+		  #FASTLY RECV
+		  remove req.http.%s;
+	  }`, tt.name)
+			assertError(t, input)
+		})
+	}
+}