feat(security): security hardening, proxy support, and GL045 HTTP include warning
Security fixes: - Path traversal guard in include: local: — paths with ../ that escape the repo root are rejected instead of reading arbitrary host files - HTTP timeout (30 s) on all fetcher requests to prevent indefinite hangs - Response size cap (10 MiB) via io.LimitReader to prevent memory exhaustion - Cache directory and file permissions tightened to 0700/0600 - LSP Content-Length cap (64 MiB) to guard against DoS from a malicious client New feature: - --proxy flag on check, graph, and lsp subcommands; also proxy: key in .glint.yml; overrides system HTTP_PROXY / HTTPS_PROXY env vars when set; cmdGraph and cmdLSP now also load .glint.yml for proxy/token/url fallbacks New lint rule: - GL045 (Warning): include: remote: using plain http:// instead of https:// Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -38,6 +38,12 @@ type Config struct {
|
||||
// CacheDir is the default directory for caching fetched remote includes.
|
||||
// Overridden by the --cache-dir flag.
|
||||
CacheDir string `yaml:"cache_dir"`
|
||||
|
||||
// Proxy is the HTTP proxy URL for fetching remote includes and GitLab API
|
||||
// calls (e.g. "http://proxy.example.com:8080"). Overridden by the --proxy
|
||||
// flag. When empty, system proxy settings (HTTP_PROXY / HTTPS_PROXY /
|
||||
// NO_PROXY env vars) are used automatically.
|
||||
Proxy string `yaml:"proxy"`
|
||||
}
|
||||
|
||||
// Load searches for a .glint.yml file starting from dir and walking up toward
|
||||
|
||||
@@ -26,10 +26,10 @@ func cacheWrite(dir, key string, data []byte) {
|
||||
if dir == "" {
|
||||
return
|
||||
}
|
||||
if err := os.MkdirAll(dir, 0o755); err != nil {
|
||||
if err := os.MkdirAll(dir, 0o700); err != nil {
|
||||
return
|
||||
}
|
||||
_ = os.WriteFile(cachePath(dir, key), data, 0o644)
|
||||
_ = os.WriteFile(cachePath(dir, key), data, 0o600)
|
||||
}
|
||||
|
||||
// cachePath returns the filesystem path for a cache entry.
|
||||
|
||||
@@ -71,3 +71,28 @@ func TestCacheWrite_MkdirAll(t *testing.T) {
|
||||
t.Errorf("directory not created: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCacheWrite_FilePermissions(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
cacheWrite(dir, "seckey", []byte("secret"))
|
||||
info, err := os.Stat(cachePath(dir, "seckey"))
|
||||
if err != nil {
|
||||
t.Fatalf("stat cache file: %v", err)
|
||||
}
|
||||
if mode := info.Mode().Perm(); mode != 0o600 {
|
||||
t.Errorf("cache file mode = %04o; want 0600", mode)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCacheWrite_DirPermissions(t *testing.T) {
|
||||
parent := t.TempDir()
|
||||
dir := filepath.Join(parent, "glint-cache")
|
||||
cacheWrite(dir, "k", []byte("v"))
|
||||
info, err := os.Stat(dir)
|
||||
if err != nil {
|
||||
t.Fatalf("stat cache dir: %v", err)
|
||||
}
|
||||
if mode := info.Mode().Perm(); mode != 0o700 {
|
||||
t.Errorf("cache dir mode = %04o; want 0700", mode)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -8,8 +8,18 @@ import (
|
||||
"net/url"
|
||||
"os"
|
||||
"strings"
|
||||
"time"
|
||||
)
|
||||
|
||||
// httpClient is the shared HTTP client for all fetcher requests.
|
||||
// Timeout guards against hung remote servers. Transport is intentionally nil
|
||||
// so http.DefaultTransport is used dynamically — allowing tests to swap it.
|
||||
var httpClient = &http.Client{Timeout: 30 * time.Second}
|
||||
|
||||
// maxResponseBytes is the per-response size cap. Responses larger than this
|
||||
// are rejected to prevent memory exhaustion from pathological servers.
|
||||
const maxResponseBytes int64 = 10 << 20 // 10 MiB
|
||||
|
||||
// TokenSource describes where a token was found, which determines the correct
|
||||
// authentication header to use with the GitLab API.
|
||||
type TokenSource int
|
||||
@@ -27,6 +37,10 @@ type GitLabConfig struct {
|
||||
Source TokenSource
|
||||
CacheDir string // local cache directory; empty = caching disabled
|
||||
Offline bool // when true, return an error instead of making network calls
|
||||
// ProxyURL overrides the HTTP proxy for all requests made with this config.
|
||||
// Empty string means use system proxy settings (HTTP_PROXY / HTTPS_PROXY /
|
||||
// NO_PROXY env vars honoured automatically by http.DefaultTransport).
|
||||
ProxyURL string
|
||||
}
|
||||
|
||||
// AutoConfig builds a GitLabConfig from environment variables.
|
||||
@@ -56,6 +70,33 @@ func AutoConfig() GitLabConfig {
|
||||
return cfg
|
||||
}
|
||||
|
||||
// WithProxy returns a copy of cfg with ProxyURL set.
|
||||
func (cfg GitLabConfig) WithProxy(proxyURL string) GitLabConfig {
|
||||
cfg.ProxyURL = proxyURL
|
||||
return cfg
|
||||
}
|
||||
|
||||
// client returns an HTTP client for this config.
|
||||
// When ProxyURL is set it takes precedence over system proxy env vars;
|
||||
// otherwise http.DefaultTransport's built-in ProxyFromEnvironment is used.
|
||||
func (cfg GitLabConfig) client() *http.Client {
|
||||
if cfg.ProxyURL == "" {
|
||||
return httpClient
|
||||
}
|
||||
proxyURL, err := url.Parse(cfg.ProxyURL)
|
||||
if err != nil {
|
||||
return httpClient
|
||||
}
|
||||
// Clone the default transport so all TLS/dial settings are preserved.
|
||||
t, ok := http.DefaultTransport.(*http.Transport)
|
||||
if !ok {
|
||||
return httpClient
|
||||
}
|
||||
transport := t.Clone()
|
||||
transport.Proxy = http.ProxyURL(proxyURL)
|
||||
return &http.Client{Timeout: httpClient.Timeout, Transport: transport}
|
||||
}
|
||||
|
||||
// WithOverrides returns a copy of cfg with the provided overrides applied.
|
||||
// Non-empty strings overwrite the corresponding field; booleans are always
|
||||
// applied (so offline: false explicitly clears offline mode).
|
||||
@@ -128,16 +169,19 @@ func (cfg GitLabConfig) FetchFile(project, filePath, ref string) ([]byte, error)
|
||||
}
|
||||
}
|
||||
|
||||
resp, err := http.DefaultClient.Do(req)
|
||||
resp, err := cfg.client().Do(req)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("GET %s: %w", apiURL, err)
|
||||
}
|
||||
defer resp.Body.Close()
|
||||
|
||||
body, err := io.ReadAll(resp.Body)
|
||||
body, err := io.ReadAll(io.LimitReader(resp.Body, maxResponseBytes+1))
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("reading response body: %w", err)
|
||||
}
|
||||
if int64(len(body)) > maxResponseBytes {
|
||||
return nil, fmt.Errorf("response body exceeds maximum size (%d MiB)", maxResponseBytes>>20)
|
||||
}
|
||||
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
msg := strings.TrimSpace(string(body))
|
||||
@@ -169,15 +213,18 @@ func (cfg GitLabConfig) FetchURL(rawURL string) ([]byte, error) {
|
||||
return nil, fmt.Errorf("offline mode: %s is not in the local cache (run without --offline first to populate the cache)", rawURL)
|
||||
}
|
||||
|
||||
resp, err := http.Get(rawURL) //nolint:noctx
|
||||
resp, err := cfg.client().Get(rawURL)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("GET %s: %w", rawURL, err)
|
||||
}
|
||||
defer resp.Body.Close()
|
||||
body, err := io.ReadAll(resp.Body)
|
||||
body, err := io.ReadAll(io.LimitReader(resp.Body, maxResponseBytes+1))
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("reading body: %w", err)
|
||||
}
|
||||
if int64(len(body)) > maxResponseBytes {
|
||||
return nil, fmt.Errorf("response body exceeds maximum size (%d MiB)", maxResponseBytes>>20)
|
||||
}
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
return nil, fmt.Errorf("GET %s: status %d", rawURL, resp.StatusCode)
|
||||
}
|
||||
|
||||
@@ -6,6 +6,7 @@ import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"os"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
@@ -339,3 +340,46 @@ func TestFetchURL_NotOK(t *testing.T) {
|
||||
_, err := cfg.FetchURL(srv.URL)
|
||||
if err == nil { t.Fatal("expected error for non-200 status") }
|
||||
}
|
||||
|
||||
func TestFetchFile_ResponseTooLarge(t *testing.T) {
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
// Write maxResponseBytes+1 bytes to exceed the cap.
|
||||
chunk := make([]byte, 4096)
|
||||
written := int64(0)
|
||||
for written <= maxResponseBytes {
|
||||
n, _ := w.Write(chunk)
|
||||
written += int64(n)
|
||||
}
|
||||
}))
|
||||
defer srv.Close()
|
||||
cfg := GitLabConfig{BaseURL: srv.URL}
|
||||
_, err := cfg.FetchFile("group/project", "/ci.yml", "main")
|
||||
if err == nil {
|
||||
t.Fatal("expected error for oversized response")
|
||||
}
|
||||
if !strings.Contains(err.Error(), "exceeds maximum size") {
|
||||
t.Errorf("unexpected error: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestFetchURL_ResponseTooLarge(t *testing.T) {
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
chunk := make([]byte, 4096)
|
||||
written := int64(0)
|
||||
for written <= maxResponseBytes {
|
||||
n, _ := w.Write(chunk)
|
||||
written += int64(n)
|
||||
}
|
||||
}))
|
||||
defer srv.Close()
|
||||
cfg := GitLabConfig{}
|
||||
_, err := cfg.FetchURL(srv.URL)
|
||||
if err == nil {
|
||||
t.Fatal("expected error for oversized response")
|
||||
}
|
||||
if !strings.Contains(err.Error(), "exceeds maximum size") {
|
||||
t.Errorf("unexpected error: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -853,4 +853,18 @@ test:
|
||||
- if: $CI_COMMIT_BRANCH == "main"
|
||||
needs: [build, lint]`,
|
||||
},
|
||||
|
||||
RuleInsecureRemoteInclude: {
|
||||
Title: "remote include uses plain HTTP",
|
||||
Severity: Warning,
|
||||
Description: "An include: remote: entry uses an http:// URL instead of https://. " +
|
||||
"CI templates fetched over plain HTTP are transmitted in cleartext; an " +
|
||||
"attacker with network access could intercept or modify the template " +
|
||||
"before it is parsed. Use https:// so the connection is encrypted and " +
|
||||
"the server's identity is verified.",
|
||||
Example: `include:
|
||||
- remote: http://ci-templates.example.com/build.yml`,
|
||||
Fix: `include:
|
||||
- remote: https://ci-templates.example.com/build.yml`,
|
||||
},
|
||||
}
|
||||
|
||||
@@ -0,0 +1,31 @@
|
||||
package linter
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"strings"
|
||||
|
||||
"git.k3nny.fr/glint/internal/model"
|
||||
)
|
||||
|
||||
// checkInsecureRemoteInclude warns when an include: remote: entry uses plain
|
||||
// HTTP (GL045). The file is still fetched and linted; the finding is a warning
|
||||
// so pipelines that use HTTP for internal infra are not blocked.
|
||||
func checkInsecureRemoteInclude(p *model.Pipeline) []Finding {
|
||||
var findings []Finding
|
||||
for _, inc := range p.Include {
|
||||
m, ok := inc.(map[string]any)
|
||||
if !ok {
|
||||
continue
|
||||
}
|
||||
remote, _ := m["remote"].(string)
|
||||
if strings.HasPrefix(remote, "http://") {
|
||||
findings = append(findings, Finding{
|
||||
Severity: Warning,
|
||||
Rule: RuleInsecureRemoteInclude,
|
||||
File: p.SourceFile,
|
||||
Message: fmt.Sprintf("remote include %q uses plain HTTP; CI templates are fetched unencrypted — prefer HTTPS", remote),
|
||||
})
|
||||
}
|
||||
}
|
||||
return findings
|
||||
}
|
||||
@@ -0,0 +1,73 @@
|
||||
package linter
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"git.k3nny.fr/glint/internal/model"
|
||||
)
|
||||
|
||||
func TestCheckInsecureRemoteInclude(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
include []any
|
||||
wantGL string // expected rule ID, empty = no findings
|
||||
}{
|
||||
{
|
||||
name: "http URL triggers GL045",
|
||||
include: []any{map[string]any{"remote": "http://example.com/ci.yml"}},
|
||||
wantGL: RuleInsecureRemoteInclude,
|
||||
},
|
||||
{
|
||||
name: "https URL is clean",
|
||||
include: []any{map[string]any{"remote": "https://example.com/ci.yml"}},
|
||||
},
|
||||
{
|
||||
name: "local include ignored",
|
||||
include: []any{map[string]any{"local": "/templates/ci.yml"}},
|
||||
},
|
||||
{
|
||||
name: "string include ignored",
|
||||
include: []any{"/templates/ci.yml"},
|
||||
},
|
||||
{
|
||||
name: "no includes",
|
||||
include: nil,
|
||||
},
|
||||
{
|
||||
name: "mixed: http fires, https does not",
|
||||
include: []any{
|
||||
map[string]any{"remote": "https://ok.example.com/ci.yml"},
|
||||
map[string]any{"remote": "http://bad.example.com/ci.yml"},
|
||||
},
|
||||
wantGL: RuleInsecureRemoteInclude,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range tests {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
p := &model.Pipeline{
|
||||
Include: tc.include,
|
||||
Jobs: map[string]model.Job{},
|
||||
}
|
||||
findings := checkInsecureRemoteInclude(p)
|
||||
if tc.wantGL == "" {
|
||||
if len(findings) != 0 {
|
||||
t.Errorf("expected no findings, got: %v", findings)
|
||||
}
|
||||
return
|
||||
}
|
||||
found := false
|
||||
for _, f := range findings {
|
||||
if f.Rule == tc.wantGL {
|
||||
found = true
|
||||
if f.Severity != Warning {
|
||||
t.Errorf("severity = %v; want Warning", f.Severity)
|
||||
}
|
||||
}
|
||||
}
|
||||
if !found {
|
||||
t.Errorf("expected finding %s, got: %v", tc.wantGL, findings)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -67,6 +67,7 @@ func Lint(p *model.Pipeline, skipped map[string]bool) []Finding {
|
||||
findings = append(findings, checkVariableRefs(p)...)
|
||||
findings = append(findings, checkRulesIfReachability(p)...)
|
||||
findings = append(findings, checkInheritCompleteness(p)...)
|
||||
findings = append(findings, checkInsecureRemoteInclude(p)...)
|
||||
slices.SortStableFunc(findings, func(a, b Finding) int {
|
||||
if c := cmp.Compare(a.File, b.File); c != 0 {
|
||||
return c
|
||||
|
||||
@@ -158,4 +158,9 @@ const (
|
||||
// GL044: a rules:needs: entry references a job that does not exist in the pipeline.
|
||||
// rules:needs: overrides the top-level needs: when a specific rule matches (GitLab CI 16.4+).
|
||||
RuleRulesNeedsUnknown = "GL044"
|
||||
|
||||
// GL045: an include: remote: entry uses plain HTTP instead of HTTPS.
|
||||
// CI templates fetched over HTTP are transmitted in cleartext and can be
|
||||
// intercepted or modified in transit.
|
||||
RuleInsecureRemoteInclude = "GL045"
|
||||
)
|
||||
|
||||
@@ -66,6 +66,11 @@ func (s *Server) Run() error {
|
||||
}
|
||||
}
|
||||
|
||||
// maxLSPMessageBytes caps the body size accepted from an LSP client.
|
||||
// A legitimate editor message is never this large; enforcing the cap prevents
|
||||
// a crafted Content-Length from triggering a multi-gigabyte allocation.
|
||||
const maxLSPMessageBytes = 64 << 20 // 64 MiB
|
||||
|
||||
// readMessage reads one Content-Length–framed JSON-RPC message from the stream.
|
||||
func (s *Server) readMessage() (*Message, error) {
|
||||
var contentLength int
|
||||
@@ -92,6 +97,9 @@ func (s *Server) readMessage() (*Message, error) {
|
||||
if contentLength == 0 {
|
||||
return nil, fmt.Errorf("missing or zero Content-Length header")
|
||||
}
|
||||
if contentLength > maxLSPMessageBytes {
|
||||
return nil, fmt.Errorf("Content-Length %d exceeds maximum %d", contentLength, maxLSPMessageBytes)
|
||||
}
|
||||
|
||||
body := make([]byte, contentLength)
|
||||
if _, err := io.ReadFull(s.in, body); err != nil {
|
||||
|
||||
@@ -364,6 +364,20 @@ func TestServer_DidClose_ClearsdiAgnostics(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestServer_ContentLengthTooLarge(t *testing.T) {
|
||||
// Craft a header with a content-length that exceeds the cap.
|
||||
// The server must reject it before allocating a giant buffer.
|
||||
header := fmt.Sprintf("Content-Length: %d\r\n\r\n", maxLSPMessageBytes+1)
|
||||
srv, _, _ := newTestServer([]byte(header))
|
||||
err := srv.Run()
|
||||
if err == nil {
|
||||
t.Fatal("expected error for oversized Content-Length, got nil")
|
||||
}
|
||||
if !strings.Contains(err.Error(), "exceeds maximum") {
|
||||
t.Errorf("unexpected error: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestServer_UriToPath(t *testing.T) {
|
||||
tests := []struct {
|
||||
uri string
|
||||
|
||||
@@ -133,6 +133,12 @@ func resolveLocalInclude(p *model.Pipeline, rawPath string, cfg fetcher.GitLabCo
|
||||
absPath := filepath.Join(rootDir, relPath)
|
||||
label := "local " + rawPath
|
||||
|
||||
// Guard against path traversal: reject any path that escapes rootDir.
|
||||
rel, err := filepath.Rel(rootDir, absPath)
|
||||
if err != nil || strings.HasPrefix(rel, "..") {
|
||||
return []IncludeWarning{{Label: label, Err: fmt.Errorf("path escapes repository root: %s", rawPath)}}, nil
|
||||
}
|
||||
|
||||
if visited[absPath] {
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
@@ -6,6 +6,7 @@ import (
|
||||
"net/http/httptest"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"git.k3nny.fr/glint/internal/fetcher"
|
||||
@@ -332,6 +333,32 @@ func TestResolveLocalInclude_WithNestedIncludes(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestResolveLocalInclude_PathTraversal(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
p := &model.Pipeline{Jobs: map[string]model.Job{}, RawJobs: map[string]map[string]any{}}
|
||||
// A path that escapes the repository root via "../.." must produce a warning,
|
||||
// not silently read an arbitrary file.
|
||||
warnings, _ := resolveLocalInclude(p, "../../etc/passwd", fetcher.GitLabConfig{}, dir, map[string]bool{}, 0)
|
||||
if len(warnings) == 0 {
|
||||
t.Fatal("expected warning for path traversal, got none")
|
||||
}
|
||||
if !strings.Contains(warnings[0].Err.Error(), "path escapes") {
|
||||
t.Errorf("unexpected warning: %v", warnings[0])
|
||||
}
|
||||
if len(p.Jobs) != 0 {
|
||||
t.Error("no jobs should be merged when path escapes root")
|
||||
}
|
||||
}
|
||||
|
||||
func TestResolveLocalInclude_PathTraversalWithLeadingSlash(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
p := &model.Pipeline{Jobs: map[string]model.Job{}, RawJobs: map[string]map[string]any{}}
|
||||
warnings, _ := resolveLocalInclude(p, "/../../etc/shadow", fetcher.GitLabConfig{}, dir, map[string]bool{}, 0)
|
||||
if len(warnings) == 0 {
|
||||
t.Fatal("expected warning for path traversal via leading slash, got none")
|
||||
}
|
||||
}
|
||||
|
||||
// ── resolveRemoteInclude ──────────────────────────────────────────────────────
|
||||
|
||||
func TestResolveRemoteInclude_Success(t *testing.T) {
|
||||
|
||||
Reference in New Issue
Block a user