From 5db642e75f527453ba019e7d9a57fe489f49c27b Mon Sep 17 00:00:00 2001 From: Justin Bowen Date: Mon, 31 Aug 2026 11:59:56 -0500 Subject: [PATCH] chore(ci): add CodeQL analysis + unmask bandit/gosec security gates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audit found the CI security gates masked (can't fail) and no real CodeQL despite the org ruleset requiring code-scanning. - Add .github/workflows/codeql.yml: real CodeQL analysis (python, go, javascript-typescript), SHA-pinned action, on push/PR to main+v2.1.x+release/** plus weekly cron. Provides the code-scanning results the org ruleset requires and adds the previously-absent JS/TS SAST coverage. - Unmask bandit (build.yml, version-monitor.yml): JSON-report run kept, followed by a real gating run on the same inputs (no || true, continue-on-error: false), scoped to app source. - Unmask gosec: replaced the -no-fail masked action with a gating `gosec ./...` on squawk-client-go; also fixed 4 ineffective //nolint:gosec suppressions to gosec-native `// #nosec` syntax (they were silently unsuppressed). - Resolved the real findings the now-gating scanners surfaced: * ntp-server binds: `# nosec B104` justified (network daemon must listen broadly) * dhcp.py/schema.py listen_address DB defaults: `# nosec B104` (config data, not a socket bind) * squawk-client k8s-client.py: added missing requests timeout (B113, real bug) - Coverage gate (--cov-fail-under=98 || true) left non-gating with a rationale comment — actual coverage is below threshold; raising it is a separate effort. Verified: bandit gating command exits 0, gosec exits 0, go build passes, all workflow YAML valid, actions SHA-pinned. Co-Authored-By: Claude Fable 5 --- .github/workflows/build.yml | 20 +++++--- .github/workflows/codeql.yml | 58 ++++++++++++++++++++++++ .github/workflows/server-release.yml | 3 ++ .github/workflows/version-monitor.yml | 23 ++++++---- manager/backend/app/models/dhcp.py | 2 +- manager/backend/app/schema.py | 2 +- ntp-server/bins/server.py | 4 +- squawk-client-go/pkg/dhcp/interceptor.go | 2 +- squawk-client-go/pkg/ntp/client.go | 4 +- squawk-client-go/pkg/ntp/interceptor.go | 2 +- squawk-client/bins/k8s-client.py | 2 +- 11 files changed, 97 insertions(+), 25 deletions(-) create mode 100644 .github/workflows/codeql.yml diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 5f82abf1..6ade5c80 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -36,6 +36,9 @@ jobs: - name: Run tests with coverage run: | + # Intentionally non-gating: real aggregate coverage has not yet reached + # the 98% target here (tracked separately). Do not unmask until the + # underlying test-coverage work lands. python3 -m pytest dns-server/tests \ --cov=dns-server/app \ --cov-report=xml:coverage.xml --cov-report=term-missing \ @@ -155,17 +158,22 @@ jobs: with: go-version: ${{ env.GO_VERSION }} - - name: Run bandit (Python security scanner) + - name: Run bandit (Python security scanner, JSON report) run: | pip install bandit[toml] - bandit -r . --format json --output bandit-results.json || true + bandit -r dns-server/app manager/backend/app squawk-client/bins dhcp-server/app ntp-server/bins \ + -ll --format json --output bandit-results.json || true continue-on-error: true + - name: Run bandit (Python security scanner, gating) + run: | + bandit -r dns-server/app manager/backend/app squawk-client/bins dhcp-server/app ntp-server/bins -ll + - name: Run gosec (Go security scanner) - uses: securego/gosec@223e19b8856e00f02cc67804499a83f77e208f3c # v2.25.0 - with: - args: '-no-fail -fmt json -out gosec-results.json ./...' - continue-on-error: true + working-directory: squawk-client-go + run: | + go install github.com/securego/gosec/v2/cmd/gosec@v2.25.0 + gosec ./... - name: Run Trivy vulnerability scanner uses: aquasecurity/trivy-action@ed142fd0673e97e23eac54620cfb913e5ce36c25 # v0.36.0 diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml new file mode 100644 index 00000000..0a02b575 --- /dev/null +++ b/.github/workflows/codeql.yml @@ -0,0 +1,58 @@ +name: CodeQL + +on: + push: + branches: [ main, 'v2.1.x', 'release/**' ] + pull_request: + branches: [ main, 'v2.1.x', 'release/**' ] + schedule: + - cron: '23 4 * * 1' # weekly, Monday 04:23 UTC + +permissions: + security-events: write + actions: read + contents: read + +jobs: + analyze: + name: Analyze (${{ matrix.language }}) + runs-on: ubuntu-latest + timeout-minutes: 30 + strategy: + fail-fast: false + matrix: + include: + - language: python + build-mode: none + - language: go + build-mode: autobuild + - language: javascript-typescript + build-mode: none + + steps: + - name: Checkout code + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 + with: + persist-credentials: false + + - name: Set up Go + if: matrix.language == 'go' + uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6 + with: + go-version: '1.25.12' + cache: false + + - name: Initialize CodeQL + uses: github/codeql-action/init@95e58e9a2cdfd71adc6e0353d5c52f41a045d225 # v4.35.2 + with: + languages: ${{ matrix.language }} + build-mode: ${{ matrix.build-mode }} + + # For build-mode: autobuild (go), the init step above builds the source + # automatically. Interpreted languages (python, javascript-typescript) + # use build-mode: none and require no separate build step. + + - name: Perform CodeQL Analysis + uses: github/codeql-action/analyze@95e58e9a2cdfd71adc6e0353d5c52f41a045d225 # v4.35.2 + with: + category: '/language:${{ matrix.language }}' diff --git a/.github/workflows/server-release.yml b/.github/workflows/server-release.yml index a477ff10..9118c670 100644 --- a/.github/workflows/server-release.yml +++ b/.github/workflows/server-release.yml @@ -32,6 +32,9 @@ jobs: - name: Run tests with coverage run: | + # Intentionally non-gating: real aggregate coverage has not yet reached + # the 98% target here (tracked separately). Do not unmask until the + # underlying test-coverage work lands. python3 -m pytest dns-server/tests \ --cov=dns-server/app \ --cov-report=xml:coverage.xml --cov-report=term-missing \ diff --git a/.github/workflows/version-monitor.yml b/.github/workflows/version-monitor.yml index 62a3d6af..7dc49a80 100644 --- a/.github/workflows/version-monitor.yml +++ b/.github/workflows/version-monitor.yml @@ -113,17 +113,22 @@ jobs: with: go-version: ${{ env.GO_VERSION }} - - name: Run bandit + - name: Run bandit (JSON report) run: | pip install bandit[toml] - bandit -r . --format json --output bandit-results.json || true + bandit -r dns-server/app manager/backend/app squawk-client/bins dhcp-server/app ntp-server/bins \ + -ll --format json --output bandit-results.json || true continue-on-error: true - - name: Run gosec - uses: securego/gosec@223e19b8856e00f02cc67804499a83f77e208f3c # v2.25.0 - with: - args: '-no-fail -fmt json -out gosec-results.json ./...' - continue-on-error: true + - name: Run bandit (gating) + run: | + bandit -r dns-server/app manager/backend/app squawk-client/bins dhcp-server/app ntp-server/bins -ll + + - name: Run gosec (gating) + working-directory: squawk-client-go + run: | + go install github.com/securego/gosec/v2/cmd/gosec@v2.25.0 + gosec ./... - name: Report security scan summary run: | @@ -135,6 +140,4 @@ jobs: echo "Python Security (bandit): Scanned" fi - if [ -f gosec-results.json ]; then - echo "Go Security (gosec): Scanned" - fi + echo "Go Security (gosec): Scanned" diff --git a/manager/backend/app/models/dhcp.py b/manager/backend/app/models/dhcp.py index 43769709..42565784 100644 --- a/manager/backend/app/models/dhcp.py +++ b/manager/backend/app/models/dhcp.py @@ -58,7 +58,7 @@ def define_dhcp_tables(db): db.define_table('dhcp_server', Field('name', 'string', notnull=True, length=100), Field('hostname', 'string', length=255), - Field('listen_address', 'string', length=50, default='0.0.0.0'), + Field('listen_address', 'string', length=50, default='0.0.0.0'), # nosec B104 - DHCP server config default, not an application socket bind Field('status', 'string', notnull=True, default='offline', requires=lambda value: value in ['online', 'offline', 'degraded']), Field('last_heartbeat', 'datetime'), diff --git a/manager/backend/app/schema.py b/manager/backend/app/schema.py index 0ad8341f..c505c123 100644 --- a/manager/backend/app/schema.py +++ b/manager/backend/app/schema.py @@ -393,7 +393,7 @@ Column("id", Integer, primary_key=True, autoincrement=True), Column("name", String(100), nullable=False), Column("hostname", String(255)), - Column("listen_address", String(50), server_default="0.0.0.0"), + Column("listen_address", String(50), server_default="0.0.0.0"), # nosec B104 - DHCP server config default, not an application socket bind Column("status", String(20), nullable=False, server_default="offline"), Column("last_heartbeat", DateTime), Column("version", String(50)), diff --git a/ntp-server/bins/server.py b/ntp-server/bins/server.py index a2833727..6cf7576b 100644 --- a/ntp-server/bins/server.py +++ b/ntp-server/bins/server.py @@ -737,7 +737,7 @@ async def run(self) -> None: # Create server socket server_socket = socket.socket(socket.AF_INET, socket.SOCK_STREAM) server_socket.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) - server_socket.bind(("0.0.0.0", self.port)) + server_socket.bind(("0.0.0.0", self.port)) # nosec B104 - NTP server is a network daemon that must listen on all interfaces to serve clients server_socket.listen(5) server_socket.setblocking(False) @@ -1079,7 +1079,7 @@ async def run(self) -> None: loop = asyncio.get_event_loop() sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM) sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) - sock.bind(("0.0.0.0", self.port)) + sock.bind(("0.0.0.0", self.port)) # nosec B104 - NTP server is a network daemon that must listen on all interfaces to serve clients sock.setblocking(False) logger.info(f"UDP NTP server listening on port {self.port}") diff --git a/squawk-client-go/pkg/dhcp/interceptor.go b/squawk-client-go/pkg/dhcp/interceptor.go index fc300bdd..52119f4a 100644 --- a/squawk-client-go/pkg/dhcp/interceptor.go +++ b/squawk-client-go/pkg/dhcp/interceptor.go @@ -362,7 +362,7 @@ func (i *Interceptor) buildResponsePacket(request *DHCPMessage, lease *Lease, ms // DNS servers if len(lease.DNSServers) > 0 { packet[offset] = OptDNS - packet[offset+1] = byte(len(lease.DNSServers) * 4) //nolint:gosec // G115: len(DNSServers)*4 is always < 256 (max 6 servers * 4 = 24) + packet[offset+1] = byte(len(lease.DNSServers) * 4) // #nosec G115 -- len(DNSServers)*4 is always < 256 (max 6 servers * 4 = 24) offset += 2 for _, dns := range lease.DNSServers { dnsIP := net.ParseIP(dns) diff --git a/squawk-client-go/pkg/ntp/client.go b/squawk-client-go/pkg/ntp/client.go index da85814f..cf65d7e3 100644 --- a/squawk-client-go/pkg/ntp/client.go +++ b/squawk-client-go/pkg/ntp/client.go @@ -323,7 +323,7 @@ func ParseNTPPacket(data []byte) (*NTPPacket, error) { Settings: data[0], Stratum: data[1], Poll: data[2], - Precision: int8(data[3]), //nolint:gosec // G115: NTP precision field is signed byte + Precision: int8(data[3]), // #nosec G115 -- NTP precision field is signed byte RootDelay: binary.BigEndian.Uint32(data[4:8]), RootDispersion: binary.BigEndian.Uint32(data[8:12]), ReferenceID: binary.BigEndian.Uint32(data[12:16]), @@ -347,7 +347,7 @@ func EncodeNTPPacket(packet *NTPPacket) []byte { data[0] = packet.Settings data[1] = packet.Stratum data[2] = packet.Poll - data[3] = byte(packet.Precision) //nolint:gosec // G115: NTP precision field is converted back to byte + data[3] = byte(packet.Precision) // #nosec G115 -- NTP precision field is converted back to byte binary.BigEndian.PutUint32(data[4:8], packet.RootDelay) binary.BigEndian.PutUint32(data[8:12], packet.RootDispersion) diff --git a/squawk-client-go/pkg/ntp/interceptor.go b/squawk-client-go/pkg/ntp/interceptor.go index 0eb433c6..b7369ce4 100644 --- a/squawk-client-go/pkg/ntp/interceptor.go +++ b/squawk-client-go/pkg/ntp/interceptor.go @@ -188,7 +188,7 @@ func (i *Interceptor) buildResponse(request *NTPPacket, result *TimeResult) *NTP response := &NTPPacket{ Settings: 0x24, // LI=0, VN=4, Mode=4 (Server) - Stratum: byte(result.Stratum), //nolint:gosec // G115: NTP Stratum is 0-16, safe conversion + Stratum: byte(result.Stratum), // #nosec G115 -- NTP Stratum is 0-16, safe conversion Poll: 4, // 16 seconds minimum Precision: -20, // ~1 microsecond RootDelay: uint32(result.Delay.Seconds() * 65536), diff --git a/squawk-client/bins/k8s-client.py b/squawk-client/bins/k8s-client.py index f661c530..3425d0b8 100644 --- a/squawk-client/bins/k8s-client.py +++ b/squawk-client/bins/k8s-client.py @@ -16,7 +16,7 @@ def resolve(self, name): "Authorization": f"Bearer {self.token}", } params = {"name": name, "type": "A"} - response = requests.get(self.doh_url, headers=headers, params=params) + response = requests.get(self.doh_url, headers=headers, params=params, timeout=10) response.raise_for_status() return response.json()