diff --git a/.clang-tidy b/.clang-tidy new file mode 100644 index 00000000..429ecdab --- /dev/null +++ b/.clang-tidy @@ -0,0 +1,36 @@ +# Shared clang-tidy baseline for dd-trace-cpp, httpd-datadog, and nginx-datadog. +# Keep this file in sync across those repositories. +Checks: > + -*, + bugprone-assert-side-effect, + bugprone-bool-pointer-implicit-conversion, + bugprone-copy-constructor-init, + bugprone-dangling-handle, + bugprone-forwarding-reference-overload, + bugprone-inaccurate-erase, + bugprone-infinite-loop, + bugprone-macro-repeated-side-effects, + bugprone-move-forwarding-reference, + bugprone-parent-virtual-call, + bugprone-posix-return, + bugprone-string-constructor, + bugprone-undelegated-constructor, + bugprone-unused-raii, + bugprone-virtual-near-miss, + misc-unused-using-decls, + readability-delete-null-pointer, + readability-redundant-control-flow, + readability-redundant-string-cstr +# Deferred until existing findings are cleaned up: +# bugprone-argument-comment +# bugprone-use-after-move +# misc-redundant-expression +# misc-static-assert +# modernize-redundant-void-arg +# performance-move-const-arg +# performance-noexcept-move-constructor +# readability-redundant-member-init +# readability-simplify-boolean-expr +WarningsAsErrors: '*' +HeaderFilterRegex: '^src/' +FormatStyle: file diff --git a/.gitlab/build-and-test-fast.yml b/.gitlab/build-and-test-fast.yml index f46b3a69..617d012b 100644 --- a/.gitlab/build-and-test-fast.yml +++ b/.gitlab/build-and-test-fast.yml @@ -59,6 +59,17 @@ lint-nginx-log-format: script: - bin/nginx-log-format-tidy.sh +lint-tidy: + extends: .build-and-test-fast + image: $NYDUS_IMAGE + tags: ["docker-in-docker:amd64"] + variables: + WAF: "ON" + NGINX_VERSION: "1.31.1" + BUILD_TYPE: "Debug" + script: + - bin/lint-tidy.sh + shellcheck: extends: .build-and-test-fast image: $MUSL_TOOLCHAIN_IMAGE diff --git a/CMakeLists.txt b/CMakeLists.txt index 373fba29..f527866f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -22,6 +22,9 @@ option(NGINX_COVERAGE "Add coverage instrumentation" OFF) option(ENABLE_FUZZERS "For building fuzzers" OFF) option(ENABLE_ASAN "Build with AddressSanitizer" OFF) option(ENABLE_MSAN "Build with MemorySanitizer" OFF) +option(NGINX_DATADOG_ENABLE_CLANG_TIDY "Run clang-tidy on first-party sources during build" OFF) +set(NGINX_DATADOG_CLANG_TIDY "clang-tidy" CACHE STRING + "clang-tidy executable when NGINX_DATADOG_ENABLE_CLANG_TIDY is ON") set(NGINX_DATADOG_FLAVOR "nginx" CACHE STRING "NGINX flavor") if(ENABLE_ASAN AND ENABLE_MSAN) @@ -199,6 +202,10 @@ target_sources(ngx_http_datadog_module PRIVATE ${CMAKE_BINARY_DIR}/version.cpp) add_library(ngx_http_datadog_objs OBJECT) set_target_properties(ngx_http_datadog_objs PROPERTIES POSITION_INDEPENDENT_CODE ON) +if(NGINX_DATADOG_ENABLE_CLANG_TIDY) + set_property(TARGET ngx_http_datadog_objs PROPERTY CXX_CLANG_TIDY + "${NGINX_DATADOG_CLANG_TIDY};--quiet;--use-color") +endif() target_compile_definitions(ngx_http_datadog_objs PUBLIC DD_NGINX_FLAVOR="${NGINX_DATADOG_FLAVOR}") target_sources(ngx_http_datadog_objs PRIVATE diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c01988f3..77e08285 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -6,11 +6,39 @@ Follow [doc/conventions.md](doc/conventions.md). ## Format -- `make lint`: check format -- `make format` fix format +- `make lint`: check clang-format and Python format +- `make format`: rewrite files to match Rebuild formatter image after editing `Dockerfile.formatter` with `make build-formatter-image`. +## Static Analysis + +C++ is analyzed with **clang-tidy-19** (pinned via Alpine's `clang19` / +`clang19-extra-tools` packages on alpine:3.23.4; the image's default `clang` +is LLVM 21) using the shared `.clang-tidy` baseline. Warnings are errors. + +Do not run clang-tidy on the host. `compile_commands.json` must be produced by +the same container that runs tidy. + +```shell +NGINX_VERSION= make lint-tidy +``` + +This runs `bin/lint-tidy.sh`, which re-execs in Docker, configures CMake with +`NGINX_DATADOG_ENABLE_CLANG_TIDY`, and builds `nginx_module`. CMake invokes +clang-tidy-19 with the exact compile line on `ngx_http_datadog_objs` only +(not `src/rum/` unless `RUM=ON`, and not `tools/`, tests, or vendored +submodules). Set `WAF=ON` (the default) to include AppSec sources. + +The custom nginx log-format plugin is separate: + +```shell +NGINX_VERSION= make lint-nginx-log-format +``` + +GitLab jobs `lint-tidy` and `lint-nginx-log-format` run the same scripts and +fail the merge request pipeline on findings. + ## Build Locally ```shell diff --git a/Makefile b/Makefile index 3d78dd1c..2345c1eb 100644 --- a/Makefile +++ b/Makefile @@ -135,6 +135,10 @@ lint: ensure-formatter-image .clang-format lint-nginx-log-format: bin/nginx-log-format-tidy.sh +.PHONY: lint-tidy +lint-tidy: + bin/lint-tidy.sh + .PHONY: ensure-formatter-image ensure-formatter-image: ifeq ($(IN_DOCKER_OR_CI),false) diff --git a/bin/lint-tidy.sh b/bin/lint-tidy.sh new file mode 100755 index 00000000..3469cee4 --- /dev/null +++ b/bin/lint-tidy.sh @@ -0,0 +1,130 @@ +#!/bin/bash +# Run the pinned clang-tidy through CMake in a container. +# +# clang-tidy must use the same compiler, flags, and sysroot as the real +# build. CMake's CXX_CLANG_TIDY on ngx_http_datadog_objs does that: it +# invokes tidy with the exact compile line after `--`. Do not use a host +# compilation database. +# +# Usage: NGINX_VERSION= make lint-tidy + +set -eo pipefail + +# Bump these together. alpine:3.23's default clang is 21; install the +# versioned LLVM 19 packages so tidy stays on 19. +alpine_version=3.23.4 +CLANG_TIDY_VERSION=19 +container_image=${NGINX_TIDY_IMAGE:-alpine:$alpine_version} +container_repo=/repo +default_build_root=".clang-tidy-build/alpine-$alpine_version" + +if [ -z "$NGINX_VERSION" ]; then + >&2 echo 'NGINX_VERSION is not set. Please set the NGINX_VERSION environment variable.' + exit 1 +fi + +if [ "$NGINX_TIDY_IN_CONTAINER" != "1" ]; then + repo_root=$(git rev-parse --show-toplevel) + + if ! command -v docker >/dev/null 2>&1; then + >&2 echo 'docker is required to run clang-tidy.' + exit 1 + fi + + exec docker run --rm -t \ + -e NGINX_TIDY_IN_CONTAINER=1 \ + -e BUILD_DIR \ + -e BUILD_TYPE \ + -e MAKE_JOB_COUNT \ + -e NGINX_VERSION \ + -e RUM \ + -e WAF \ + -v "$repo_root:$container_repo" \ + -w "$container_repo" \ + "$container_image" \ + sh -c 'apk add --no-cache bash >/dev/null && exec bash "$@"' \ + _ "$container_repo/bin/lint-tidy.sh" "$@" +fi + +apk add --no-cache \ + ca-certificates \ + "clang${CLANG_TIDY_VERSION}" \ + "clang${CLANG_TIDY_VERSION}-extra-tools" \ + cmake \ + git \ + ninja \ + pcre2-dev \ + zlib-dev + +first_cmd() { + for candidate in "$@"; do + if [ -x "$candidate" ]; then + echo "$candidate" + return 0 + fi + if command -v "$candidate" >/dev/null 2>&1; then + command -v "$candidate" + return 0 + fi + done + return 1 +} + +llvm_bin="/usr/lib/llvm${CLANG_TIDY_VERSION}/bin" +c_compiler=$(first_cmd \ + "clang-${CLANG_TIDY_VERSION}" \ + "${llvm_bin}/clang") +compiler=$(first_cmd \ + "clang++-${CLANG_TIDY_VERSION}" \ + "${llvm_bin}/clang++") +tidy=$(first_cmd \ + "clang-tidy-${CLANG_TIDY_VERSION}" \ + "${llvm_bin}/clang-tidy") +if [ -z "$compiler" ] || [ -z "$tidy" ] || [ -z "$c_compiler" ]; then + >&2 echo "clang-${CLANG_TIDY_VERSION} / clang-tidy-${CLANG_TIDY_VERSION} not found after apk add." + exit 1 +fi + +export CC="$c_compiler" +export CXX="$compiler" + +tidy_major=$("$tidy" --version | sed -n 's/.*version \([0-9][0-9]*\).*/\1/p' | head -n 1) +compiler_major=$("$compiler" -dumpversion | cut -d. -f1) +if [ "$compiler_major" != "$CLANG_TIDY_VERSION" ] || [ "$tidy_major" != "$CLANG_TIDY_VERSION" ]; then + >&2 echo "clang-tidy and the CMake compiler must both be LLVM ${CLANG_TIDY_VERSION}." + >&2 echo " $compiler: ${compiler_major}" + >&2 echo " $tidy: ${tidy_major}" + exit 1 +fi + +jobs=${MAKE_JOB_COUNT:-} +if [ -z "$jobs" ]; then + jobs=$(getconf _NPROCESSORS_ONLN 2>/dev/null || echo 2) +fi + +build_dir=${BUILD_DIR:-$default_build_root/project} +nginx_version=$NGINX_VERSION +build_type=${BUILD_TYPE:-Debug} +waf=${WAF:-ON} +rum=${RUM:-OFF} + +case "$build_dir" in + /*) ;; + *) build_dir="$container_repo/$build_dir" ;; +esac + +# CXX_CLANG_TIDY is attached only to ngx_http_datadog_objs. Building +# nginx_module compiles first-party src/ with the exact compile line +# (RUM off by default, so src/rum/ is not a source of that target). +cmake -S "$container_repo" -B "$build_dir" -G Ninja \ + -DCMAKE_C_COMPILER="$c_compiler" \ + -DCMAKE_CXX_COMPILER="$compiler" \ + -DNGINX_VERSION="$nginx_version" \ + -DBUILD_TESTING=OFF \ + -DCMAKE_BUILD_TYPE="$build_type" \ + -DNGINX_DATADOG_ASM_ENABLED="$waf" \ + -DNGINX_DATADOG_RUM_ENABLED="$rum" \ + -DNGINX_DATADOG_ENABLE_CLANG_TIDY=ON \ + -DNGINX_DATADOG_CLANG_TIDY="$tidy" + +cmake --build "$build_dir" --target nginx_module -j "$jobs" diff --git a/src/security/.clang-tidy b/src/security/.clang-tidy index 8d1a6258..9642f36b 100644 --- a/src/security/.clang-tidy +++ b/src/security/.clang-tidy @@ -1,17 +1,4 @@ -Checks: 'readability-identifier-naming' -CheckOptions: - - { key: readability-identifier-naming.NamespaceCase , value: lower_case } - - { key: readability-identifier-naming.ClassCase , value: CamelCase } - - { key: readability-identifier-naming.StructCase , value: CamelCase } - - { key: readability-identifier-naming.FunctionCase , value: lower_case } - - { key: readability-identifier-naming.VariableCase , value: lower_case } - - { key: readability-identifier-naming.PublicMemberCase , value: lower_case } - - { key: readability-identifier-naming.PrivateMemberCase , value: lower_case } - - { key: readability-identifier-naming.PrivateMemberSuffix , value: _ } - - { key: readability-identifier-naming.ProtectedMemberCase , value: lower_case } - - { key: readability-identifier-naming.ProtectedMemberSuffix , value: _ } - - { key: readability-identifier-naming.GlobalConstantCase , value: CamelCase } - - { key: readability-identifier-naming.GlobalConstantPrefix , value: k } - - { key: readability-identifier-naming.LocalConstantCase , value: lower_case } - - { key: readability-identifier-naming.EnumConstantCase , value: UPPER_CASE } - - { key: readability-identifier-naming.TypeAliasCase , value: CamelCase } +InheritParentConfig: true +# Naming matches doc/conventions.md but is not CI-gated yet; existing +# identifiers fail readability-identifier-naming. +Checks: '-readability-identifier-naming'