From f79c64cd447a680a0e3304209291ec6c3d836d88 Mon Sep 17 00:00:00 2001 From: k3nny Date: Fri, 26 Jun 2026 21:52:57 +0200 Subject: [PATCH] feat(security): security hardening, proxy support, and GL045 HTTP include warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- CHANGELOG.md | 20 ++++++++ FEATURES.md | 10 +++- README.md | 8 +-- ROADMAP.md | 12 +++++ Taskfile.yml | 2 + cmd/glint/lsp.go | 35 +++++++++++-- cmd/glint/main.go | 32 ++++++++++-- internal/config/config.go | 6 +++ internal/fetcher/cache.go | 4 +- internal/fetcher/cache_test.go | 25 ++++++++++ internal/fetcher/gitlab.go | 55 +++++++++++++++++++-- internal/fetcher/gitlab_test.go | 44 +++++++++++++++++ internal/linter/explain.go | 14 ++++++ internal/linter/includes.go | 31 ++++++++++++ internal/linter/includes_test.go | 73 ++++++++++++++++++++++++++++ internal/linter/linter.go | 1 + internal/linter/rules.go | 5 ++ internal/lsp/server.go | 8 +++ internal/lsp/server_test.go | 14 ++++++ internal/resolver/includes.go | 6 +++ internal/resolver/includes_test.go | 27 ++++++++++ testdata/insecure_remote_include.yml | 8 +++ 22 files changed, 422 insertions(+), 18 deletions(-) create mode 100644 internal/linter/includes.go create mode 100644 internal/linter/includes_test.go create mode 100644 testdata/insecure_remote_include.yml diff --git a/CHANGELOG.md b/CHANGELOG.md index c1a4229..5044546 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,26 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). This project uses [Semantic Versioning](https://semver.org). +## [0.2.31] - 2026-06-26 + +### Added + +- **Proxy support** — `--proxy ` flag on `glint check`, `glint graph`, and `glint lsp`; also configurable via `proxy:` in `.glint.yml`. When set it takes precedence over `HTTP_PROXY` / `HTTPS_PROXY` env vars; when unset, system proxy settings are honoured automatically. Covers all remote include fetches and GitLab API calls. + +- **GL045: HTTP remote include warning** — new pipeline-level lint rule that warns when `include: remote:` uses a plain `http://` URL. CI templates fetched over unencrypted HTTP are at risk of in-transit tampering; `https://` is always preferred. + +### Fixed + +- **Path traversal in local includes** — `include: local:` paths containing `../` sequences that would escape the repository root (e.g. `../../etc/passwd`) are now rejected with a warning instead of reading arbitrary files from the host. + +- **HTTP timeout on remote fetches** — all HTTP calls in `internal/fetcher` now use a 30-second timeout; previously the client had no timeout and could hang indefinitely on slow or unresponsive servers. + +- **Unbounded response size** — remote include and GitLab API responses are now capped at 10 MiB using `io.LimitReader`; previously an arbitrarily large response could exhaust process memory. + +- **Cache file permissions** — the cache directory is created with mode `0700` (was `0755`) and cache files with `0600` (was `0644`), preventing other local users from reading cached GitLab tokens or pipeline content. + +- **LSP Content-Length DoS** — `glint lsp` now rejects incoming messages whose `Content-Length` header exceeds 64 MiB, preventing memory exhaustion from a malicious or misbehaving LSP client. + ## [0.2.30] - 2026-06-26 ### Added diff --git a/FEATURES.md b/FEATURES.md index 713ba61..38be482 100644 --- a/FEATURES.md +++ b/FEATURES.md @@ -7,7 +7,7 @@ For planned work see [ROADMAP.md](ROADMAP.md). ## Lint rules -Every finding carries a stable rule ID (`GL001` – `GL043`) that can be used to +Every finding carries a stable rule ID (`GL001` – `GL045`) that can be used to suppress, filter, or look up the check. Run `glint explain ` for a description, bad-YAML example, and fix. @@ -19,6 +19,7 @@ description, bad-YAML example, and fix. | GL002 | ERR | `workflow.rules[*].when` must be `always` or `never` | | GL036 | ERR | `default.timeout` is not a valid GitLab CI duration string | | GL040 | WARN | A stage name appears more than once in `stages:` | +| GL045 | WARN | `include: remote:` uses plain `http://` — CI templates fetched unencrypted; prefer `https://` | ### Job structure @@ -243,9 +244,14 @@ url: https://gitlab.example.com # Default cache directory. cache_dir: ~/.cache/glint + +# HTTP proxy for remote includes and GitLab API calls. +# Overrides HTTP_PROXY / HTTPS_PROXY env vars when set. +# Leave empty to use system proxy settings. +proxy: http://proxy.example.com:8080 ``` -**Priority chain:** `--token`/`--gitlab-url` flags > `.glint.yml` > environment variables. +**Priority chain:** `--token`/`--gitlab-url`/`--proxy` flags > `.glint.yml` > environment variables. ### Inline suppression (`# glint: ignore`) diff --git a/README.md b/README.md index 69b2076..0717e36 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@

License - Release + Release

> **Disclaimer:** This tool was built through iterative AI-assisted development with [Claude](https://claude.ai). It is experimental, incomplete, and not intended for production use. Coverage of GitLab CI keywords is best-effort and may lag behind GitLab's evolving spec. Use it at your own discretion — no correctness guarantees are made. Contributions and bug reports are welcome. @@ -15,11 +15,11 @@ A local tool to validate and lint `.gitlab-ci.yml` pipelines without needing a G ## What it does -- **Lints** — 43 rules covering pipeline structure, keyword constraints, `needs:`/`dependencies:` graphs, expression reachability, and deprecations (GL001–GL043); run `glint explain ` for any rule -- **Resolves includes** — local files, HTTPS URLs, GitLab project templates, and CI/CD Catalog components, with offline cache support +- **Lints** — 45 rules covering pipeline structure, keyword constraints, `needs:`/`dependencies:` graphs, expression reachability, and deprecations (GL001–GL045); run `glint explain ` for any rule +- **Resolves includes** — local files, HTTPS URLs, GitLab project templates, and CI/CD Catalog components, with offline cache support and HTTP proxy support (`--proxy` flag or `proxy:` in `.glint.yml`) - **Simulates context** — `--branch`, `--tag`, `--source` flags evaluate `rules:if:` and `only`/`except` to show which jobs would be active, manual, or skipped; `--context branch=main --context branch=develop` prints a multi-column comparison table across multiple contexts in one run - **Multiple output formats** — `--format text` (default, ruff-style), `json`, `sarif` (GitHub Code Scanning / GitLab SAST), `junit`, `github` (PR annotations) -- **Project config** — `.glint.yml` for rule suppression, severity overrides, token/URL defaults; `# glint: ignore RULE` for per-job inline suppression +- **Project config** — `.glint.yml` for rule suppression, severity overrides, token/URL/proxy defaults; `# glint: ignore RULE` for per-job inline suppression - **Graph visualization** — `glint graph` prints a terminal job tree; `glint graph pipeline` renders a GitLab CI-style SVG/PNG; `--format mermaid` emits a Mermaid flowchart; `--format html` produces a self-contained HTML file with pan/zoom and a job-detail sidebar; context flags grey out skipped jobs - **LSP server** — `glint lsp` starts a Language Server Protocol server over stdin/stdout; connect with any LSP client to get inline diagnostics (rule ID as code, error/warning severity) in VS Code, Neovim, Emacs, JetBrains, etc. - **VS Code extension** — `editors/vscode/` wraps the LSP server; inline squiggles for every glint rule directly in the editor diff --git a/ROADMAP.md b/ROADMAP.md index 7761407..9ad1e80 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -106,6 +106,18 @@ The SVG renderer and terminal tree cover the basic layout. These would bring it - [x] **`.glint.yml` config file** — shipped v0.2.19; `ignore:`, `severity:`, `stages:`, `token:`, `url:`, `cache_dir:`; searched from the pipeline directory up to the git root - [x] **Inline suppression comments** — shipped v0.2.19; `# glint: ignore GL007` before a job definition; comma/space-separated rules; `# glint: ignore all` wildcard +- [x] **Proxy support** — shipped v0.2.31; `--proxy` flag on `check`, `graph`, and `lsp` subcommands; also configurable via `proxy:` in `.glint.yml`; overrides `HTTP_PROXY` / `HTTPS_PROXY` env vars when set + +--- + +## Security & hardening + +- [x] **Path traversal guard for local includes** — shipped v0.2.31; `include: local:` paths containing `../../` or similar sequences are rejected instead of reading files outside the repository root +- [x] **HTTP timeout on remote fetches** — shipped v0.2.31; all HTTP calls via the fetcher use a 30-second timeout to prevent indefinite hangs on slow or unresponsive servers +- [x] **Unbounded response size cap** — shipped v0.2.31; remote include and GitLab API responses are capped at 10 MiB; responses exceeding the limit are rejected to prevent memory exhaustion +- [x] **Cache file permissions** — shipped v0.2.31; cache directories are created with mode `0700` and cache files with mode `0600`; previously used world-readable `0755`/`0644` +- [x] **GL045: HTTP remote include warning** — shipped v0.2.31; warns when `include: remote:` uses a plain `http://` URL; CI templates fetched unencrypted are at risk of tampering +- [x] **LSP Content-Length DoS cap** — shipped v0.2.31; `glint lsp` rejects messages with `Content-Length` exceeding 64 MiB to prevent memory exhaustion from a malicious client --- diff --git a/Taskfile.yml b/Taskfile.yml index f2299ac..aa38112 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -123,6 +123,8 @@ tasks: ignore_error: false - cmd: ./{{.BINARY}} explain GL044 ignore_error: false + - cmd: ./{{.BINARY}} check testdata/insecure_remote_include.yml + ignore_error: false - cmd: ./{{.BINARY}} explain ignore_error: false diff --git a/cmd/glint/lsp.go b/cmd/glint/lsp.go index 33f35c3..c669e8d 100644 --- a/cmd/glint/lsp.go +++ b/cmd/glint/lsp.go @@ -5,6 +5,7 @@ import ( "fmt" "os" + "git.k3nny.fr/glint/internal/config" "git.k3nny.fr/glint/internal/fetcher" "git.k3nny.fr/glint/internal/lsp" ) @@ -15,6 +16,7 @@ func cmdLSP(args []string) { gitlabURL := fs.String("gitlab-url", "", "GitLab instance URL (overrides CI_SERVER_URL / GITLAB_URL)") cacheDir := fs.String("cache-dir", "", "directory for caching fetched remote includes") offline := fs.Bool("offline", false, "skip all network calls; serve only from --cache-dir") + proxy := fs.String("proxy", "", "HTTP proxy URL for remote includes and GitLab API calls (e.g. http://proxy:8080); overrides system proxy env vars") fs.Usage = func() { fmt.Fprintf(os.Stderr, "glint %s\n\n", version) fmt.Fprint(os.Stderr, `Start a Language Server Protocol server for .gitlab-ci.yml files. @@ -42,6 +44,12 @@ Options: Do not make any network calls; resolve only local includes. Implies --cache-dir default (~/.cache/glint) when not set. + --proxy + HTTP proxy URL for remote includes and GitLab API calls + (e.g. http://proxy:8080). Overrides system proxy env vars + (HTTP_PROXY / HTTPS_PROXY). Also configurable via proxy: in + .glint.yml. + -h, --help Print help @@ -49,19 +57,40 @@ Examples: glint lsp glint lsp --token glpat-xxxx --cache-dir ~/.cache/glint glint lsp --offline + glint lsp --proxy http://proxy.example.com:8080 `) } _ = fs.Parse(args) + // Load project config from the working directory (the project root from + // which the LSP server is launched). CLI flags take priority. + wd, _ := os.Getwd() + glintCfg, cfgErr := config.Load(wd) + if cfgErr != nil { + fmt.Fprintf(os.Stderr, "glint lsp: [warning] %s: %v\n", config.Filename, cfgErr) + } + + fetcherToken := *token + if fetcherToken == "" { + fetcherToken = glintCfg.Token + } + fetcherURL := *gitlabURL + if fetcherURL == "" { + fetcherURL = glintCfg.URL + } resolvedCacheDir := *cacheDir + if resolvedCacheDir == "" { + resolvedCacheDir = glintCfg.CacheDir + } if resolvedCacheDir == "" { resolvedCacheDir = defaultCacheDir() } - if *offline && resolvedCacheDir == "" { - resolvedCacheDir = defaultCacheDir() + resolvedProxy := *proxy + if resolvedProxy == "" { + resolvedProxy = glintCfg.Proxy } - cfg := fetcher.AutoConfig().WithOverrides(*gitlabURL, *token, resolvedCacheDir, *offline) + cfg := fetcher.AutoConfig().WithOverrides(fetcherURL, fetcherToken, resolvedCacheDir, *offline).WithProxy(resolvedProxy) srv := lsp.New(os.Stdin, os.Stdout, cfg, version) if err := srv.Run(); err != nil { diff --git a/cmd/glint/main.go b/cmd/glint/main.go index a128fa3..764db8c 100644 --- a/cmd/glint/main.go +++ b/cmd/glint/main.go @@ -119,6 +119,7 @@ func cmdCheck(args []string) { gitlabURL := fs.String("gitlab-url", "", "GitLab instance URL (overrides CI_SERVER_URL / GITLAB_URL)") cacheDir := fs.String("cache-dir", "", "directory to cache fetched remote includes (created if needed)") offline := fs.Bool("offline", false, "skip all network calls; serve only from --cache-dir") + proxy := fs.String("proxy", "", "HTTP proxy URL for remote includes and GitLab API calls (e.g. http://proxy:8080); overrides system proxy env vars") format := fs.String("format", "text", "output format: text, json, sarif, junit, github") branch := fs.String("branch", "", "simulate a branch push (sets CI_COMMIT_BRANCH, …)") tag := fs.String("tag", "", "simulate a tag push (sets CI_COMMIT_TAG, …)") @@ -286,8 +287,12 @@ Examples: if *offline && resolvedCacheDir == "" { resolvedCacheDir = defaultCacheDir() } + resolvedProxy := *proxy + if resolvedProxy == "" { + resolvedProxy = glintCfg.Proxy + } - cfg := fetcher.AutoConfig().WithOverrides(fetcherURL, fetcherToken, resolvedCacheDir, *offline) + cfg := fetcher.AutoConfig().WithOverrides(fetcherURL, fetcherToken, resolvedCacheDir, *offline).WithProxy(resolvedProxy) p, err := model.Parse(path) if err != nil { @@ -448,6 +453,7 @@ func cmdGraph(args []string) { gitlabURL := fs.String("gitlab-url", "", "GitLab instance URL (overrides CI_SERVER_URL / GITLAB_URL)") cacheDir := fs.String("cache-dir", "", "directory to cache fetched remote includes (created if needed)") offline := fs.Bool("offline", false, "skip all network calls; serve only from --cache-dir") + proxy := fs.String("proxy", "", "HTTP proxy URL for remote includes and GitLab API calls (e.g. http://proxy:8080)") out := fs.String("out", "glint-out", "output directory for rendered graph files (pipeline mode)") format := fs.String("format", "svg", "pipeline output format: svg, mermaid, or html") fs.Usage = func() { @@ -559,13 +565,34 @@ Examples: return } path := fs.Arg(0) + rootDir := filepath.Dir(filepath.Clean(path)) + glintCfg, cfgErr := config.Load(rootDir) + if cfgErr != nil { + fmt.Fprintf(os.Stderr, "%s: [warning] %s: %v\n", path, config.Filename, cfgErr) + } + + fetcherToken := *token + if fetcherToken == "" { + fetcherToken = glintCfg.Token + } + fetcherURL := *gitlabURL + if fetcherURL == "" { + fetcherURL = glintCfg.URL + } resolvedCacheDir := *cacheDir + if resolvedCacheDir == "" { + resolvedCacheDir = glintCfg.CacheDir + } if *offline && resolvedCacheDir == "" { resolvedCacheDir = defaultCacheDir() } + resolvedProxy := *proxy + if resolvedProxy == "" { + resolvedProxy = glintCfg.Proxy + } - cfg := fetcher.AutoConfig().WithOverrides(*gitlabURL, *token, resolvedCacheDir, *offline) + cfg := fetcher.AutoConfig().WithOverrides(fetcherURL, fetcherToken, resolvedCacheDir, *offline).WithProxy(resolvedProxy) p, err := model.Parse(path) if err != nil { @@ -574,7 +601,6 @@ Examples: return } - rootDir := filepath.Dir(filepath.Clean(path)) resolver.ResolveIncludes(p, cfg, rootDir) //nolint:errcheck resolver.Resolve(p) //nolint:errcheck diff --git a/internal/config/config.go b/internal/config/config.go index 5bda0f3..308caac 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -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 diff --git a/internal/fetcher/cache.go b/internal/fetcher/cache.go index 7b992ab..a131197 100644 --- a/internal/fetcher/cache.go +++ b/internal/fetcher/cache.go @@ -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. diff --git a/internal/fetcher/cache_test.go b/internal/fetcher/cache_test.go index 19cc8a3..3680e8d 100644 --- a/internal/fetcher/cache_test.go +++ b/internal/fetcher/cache_test.go @@ -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) + } +} diff --git a/internal/fetcher/gitlab.go b/internal/fetcher/gitlab.go index d28d1ba..b25bcca 100644 --- a/internal/fetcher/gitlab.go +++ b/internal/fetcher/gitlab.go @@ -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) } diff --git a/internal/fetcher/gitlab_test.go b/internal/fetcher/gitlab_test.go index ef2028d..47c242a 100644 --- a/internal/fetcher/gitlab_test.go +++ b/internal/fetcher/gitlab_test.go @@ -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) + } +} diff --git a/internal/linter/explain.go b/internal/linter/explain.go index 67c2139..2ab9c48 100644 --- a/internal/linter/explain.go +++ b/internal/linter/explain.go @@ -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`, + }, } diff --git a/internal/linter/includes.go b/internal/linter/includes.go new file mode 100644 index 0000000..41fd81d --- /dev/null +++ b/internal/linter/includes.go @@ -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 +} diff --git a/internal/linter/includes_test.go b/internal/linter/includes_test.go new file mode 100644 index 0000000..fbf0720 --- /dev/null +++ b/internal/linter/includes_test.go @@ -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) + } + }) + } +} diff --git a/internal/linter/linter.go b/internal/linter/linter.go index af87400..b14a0c6 100644 --- a/internal/linter/linter.go +++ b/internal/linter/linter.go @@ -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 diff --git a/internal/linter/rules.go b/internal/linter/rules.go index aed68bb..4a625a2 100644 --- a/internal/linter/rules.go +++ b/internal/linter/rules.go @@ -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" ) diff --git a/internal/lsp/server.go b/internal/lsp/server.go index edd44cf..648b2da 100644 --- a/internal/lsp/server.go +++ b/internal/lsp/server.go @@ -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 { diff --git a/internal/lsp/server_test.go b/internal/lsp/server_test.go index fe749ff..be5e750 100644 --- a/internal/lsp/server_test.go +++ b/internal/lsp/server_test.go @@ -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 diff --git a/internal/resolver/includes.go b/internal/resolver/includes.go index 6e81dcf..4dd419f 100644 --- a/internal/resolver/includes.go +++ b/internal/resolver/includes.go @@ -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 } diff --git a/internal/resolver/includes_test.go b/internal/resolver/includes_test.go index 4646ba6..0664e37 100644 --- a/internal/resolver/includes_test.go +++ b/internal/resolver/includes_test.go @@ -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) { diff --git a/testdata/insecure_remote_include.yml b/testdata/insecure_remote_include.yml new file mode 100644 index 0000000..fb8e2c0 --- /dev/null +++ b/testdata/insecure_remote_include.yml @@ -0,0 +1,8 @@ +stages: [build] + +include: + - remote: http://ci-templates.example.invalid/template.yml + +build-job: + stage: build + script: echo hello