FXC-3603: security fixes in github actions as suggested by zizmor

[mahlau-flex]

Fixed all (except one) of the many security vulnerabilities found by zizmor (12 informational, 0 low, 19 medium, 19 high). While I am pretty sure that the functionality remains the same, we will need to see (an carefully review) if something broke during these changes.

The one issue I could not change is the following (status informational):

info[use-trusted-publishing]: prefer trusted publishing for authentication
   --> .github/workflows/tidy3d-python-client-release.yml:121:9
    |
119 |       run: |
    |       --- this step
120 |         python -m build
121 |         python -m twine upload --repository pypi dist/*
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this command
    |
    = note: audit confidence → High

I have suppressed the zizmor warning for now. In the long run it would probably be best to use the newer authentication of pypi releases (according to Claude, one can just register a github workflow in pypi instead of using the legacy token system).

Additionally, I have added a pre-commit check for zizmor as well. This might seem a bit excessive, but runs extremely quickly so I think it will not bother anyone.

Lastly, I changed the configuration of zizmor itself in the github actions. I noticed that in the last PR, the job completed successfully, even though it found a security issue. This is, because it uploads the results to github advanced security, which then makes a comment showing the issue. But, this does not prevent merging and is not clearly visibly. With the new system, the checks themselves should (hopefully) fail.

Greptile Overview

Updated On: 2025-10-09 19:03:13 UTC

Summary

This PR implements comprehensive security hardening for GitHub Actions workflows based on findings from the zizmor security scanner, which identified 50 security vulnerabilities (12 informational, 19 medium, 19 high) across the CI/CD pipeline. The changes follow GitHub Actions security best practices without altering functionality.

Key security improvements include:

1. Permission Management: Moving from broad permissions to the principle of least privilege by setting global permissions to contents: read and granting specific permissions only where needed (e.g., contents: write for jobs that push changes)

2. Action Security: Pinning action versions to specific commit SHAs instead of floating tags to prevent supply chain attacks where malicious code could be injected through compromised action updates

3. Credential Protection: Adding persist-credentials: false to checkout actions to prevent GitHub tokens from being accessible to subsequent workflow steps that don't need them

4. Script Injection Prevention: Moving GitHub context expressions (like PR titles and branch names) to environment variables to prevent potential script injection attacks

5. Proactive Security: Adding zizmor as both a dependency in pyproject.toml and a pre-commit hook to catch future security issues during development

The changes span 8 workflow files covering testing, releases, documentation sync, and daily operations. One informational issue regarding PyPI trusted publishing was acknowledged but left unaddressed, with plans to migrate to the newer authentication system in the future. The implementation maintains all existing functionality while significantly reducing the attack surface of the CI/CD pipeline.

Important Files Changed

Changed Files

Filename	Score	Overview
.github/workflows/tidy3d-python-client-tests.yml	4/5	Comprehensive security hardening with action pinning, credential protection, and zizmor integration
.github/workflows/tidy3d-python-client-release.yml	4/5	Security improvements for release workflow with permission scoping and action pinning
.github/workflows/tidy3d-python-client-update-lockfile.yml	4/5	Security hardening for lockfile update workflow with SHA pinning and permission controls
.github/workflows/tidy3d-python-client-daily.yml	4/5	Permission restrictions and explicit secret passing for daily workflow security
.github/workflows/tidy3d-docs-sync-readthedocs-repo.yml	5/5	Clean security improvements with minimal permission grants for documentation sync
.github/workflows/tidy3d-python-client-submodules-test.yml	5/5	Simple security hardening with read-only permissions and credential protection
.github/workflows/tidy3d-python-client-develop-cli.yml	5/5	Standard security improvements with action pinning and permission restrictions
.pre-commit-config.yaml	4/5	Addition of zizmor pre-commit hook for proactive security scanning
pyproject.toml	5/5	Clean addition of zizmor dependency for security tooling integration

Confidence score: 4/5

• This PR is safe to merge with careful monitoring of CI/CD pipeline functionality
• Score reflects the comprehensive nature of security changes across critical workflow files that require validation
• Pay close attention to all workflow files to ensure CI/CD operations continue functioning correctly

Sequence Diagram

sequenceDiagram
    participant User
    participant Dev as "Developer"
    participant GitHub as "GitHub Actions"
    participant PyPI
    participant ReadTheDocs as "ReadTheDocs Mirror"
    participant Zizmor as "Zizmor Security Tool"
    
    User->>Dev: "Request security fixes for GitHub Actions"
    Dev->>Zizmor: "Run security audit on .github/workflows/*"
    Zizmor-->>Dev: "Report 12 informational, 19 medium, 19 high vulnerabilities"
    
    Dev->>GitHub: "Update workflow files with security fixes"
    Note over Dev,GitHub: Pin action versions to specific commit hashes<br/>Add persist-credentials: false<br/>Set minimal permissions<br/>Update to latest action versions
    
    Dev->>GitHub: "Add zizmor pre-commit hook"
    GitHub->>Zizmor: "Run zizmor check on workflow files"
    Zizmor-->>GitHub: "Security validation results"
    
    Dev->>GitHub: "Configure zizmor to fail CI on security issues"
    Note over GitHub: Previously uploaded to Advanced Security<br/>Now fails the check directly
    
    GitHub->>PyPI: "Release workflow with suppressed trusted publishing warning"
    Note over GitHub,PyPI: Legacy token authentication maintained<br/>zizmor warning suppressed
    
    GitHub->>ReadTheDocs: "Sync documentation with security fixes"
    
    Dev->>User: "Security fixes complete (except trusted publishing)"
    Note over User,Dev: All issues fixed except one informational<br/>about using trusted publishing for PyPI

Loading

[greptile-apps]

_{9 files reviewed, 4 comments}

_{Edit Code Review Agent Settings | Greptile}

[github-actions]

Diff Coverage

Diff: origin/develop...HEAD, staged and unstaged changes

No lines with coverage information in this diff.

[mahlau-flex]

I have renamed the title of this PR to link to the Zizmor jira task. Are we supposed to link multiple PRs to a single jira issue or should we create a new one everytime?

[mahlau-flex]

One thing I should add: This if-case was always true in the old version due to how github actions works. The new version implements the correct check. It is worthwile to discuss, if we want to remove the if-clause completely, because previously we seemed to test on windows anyways.

[yaugenst-flex]

@daquinteroflex do you know what the reasoning there was? I'd tend toward just removing it, as @mahlau-flex is suggesting.

[yaugenst-flex]

Thanks @mahlau-flex this is great! Besides what was already commented LGTM. I'm not too familiar with the details but as long as everything keeps running it looks fine to me.

[yaugenst-flex]

Could you explain this change? Wouldn't this mean that the scope is now missing from GITHUB_TOKEN, but there are workflow steps like check-current-approval-status that do require access to the PR? Would pull-requests: read do the trick?

[mahlau-flex]

The problem with this line was, that not all of the jobs need the write permission. Zizmor declared this as a "too broad" permission, so I moved the write permission only to the jobs that actually need it (see line 421 and 520).