Skip to content

add config-secrets option to allow-list secrets in workflows#649

Open
tdeo wants to merge 2 commits into
rhysd:mainfrom
tdeo:feature/config-secrets
Open

add config-secrets option to allow-list secrets in workflows#649
tdeo wants to merge 2 commits into
rhysd:mainfrom
tdeo:feature/config-secrets

Conversation

@tdeo
Copy link
Copy Markdown

@tdeo tdeo commented Apr 15, 2026

Fixes #609

I did use claude-code to open this because I'm not familiar at all with go. I tried to review the changes as best I could but apologize in advance if this is not good enough.

Summary

  • Adds a new config-secrets configuration option in actionlint.yaml, parallel to the existing config-variables option
  • When set to an array of secret names, actionlint reports any secrets.XXXX access where XXXX is not in the list (case-insensitive)
  • Default value null disables the check; an empty array disallows all secrets
  • Updates docs/config.md with documentation and usage example

Test plan

  • New project-level integration test in testdata/projects/config_secrets/
  • New unit tests in TestExprSemanticsCheckOK for secret, known secret, and secret name is case insensitive
  • New unit tests in TestExprSemanticsCheckError for no secret is allowed and unknown secret
  • All existing tests pass (go test ./...)
  • staticcheck ./... passes cleanly

Introduces a new `config-secrets` configuration option in `actionlint.yaml`,
parallel to the existing `config-variables` option. When set to an array of
secret names, actionlint will report any `secrets.XXXX` access where XXXX is
not in the list. The default value `null` disables the check; an empty array
disallows all secrets.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@rhysd
Copy link
Copy Markdown
Owner

rhysd commented Apr 19, 2026

Since this change seems to be generated by Claude, let me ask a question. Did you review the generated code, confirmed the test results, and validated the behavior with actual config files?

Comment thread expr_sema.go
n.Property,
)
return
}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this fast path is necessary because the below error message is comprehensive enough.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we should also remove the same fast path from checkConfigVariables just above?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rhysd do you have any opinion whether I should handle checkConfigVariables at the same time?

@tdeo
Copy link
Copy Markdown
Author

tdeo commented Apr 20, 2026

Since this change seems to be generated by Claude, let me ask a question. Did you review the generated code, confirmed the test results, and validated the behavior with actual config files?

Hello, that's a very good, while I didn't know anything about go and not much about this project's internal, what I did review myself was:

  • that the generated files under testdata/ made sense, that they were implemented in a very similar way to what exists for config variables and that the output is the behavior I would expect for a project with that workflow and configuration
  • that the implementation of checkConfigSecrets seemed to be coherent with what existed for checkConfigVariables
  • that the test suite still worked locally (claude was able to run it by itself)
  • I did see that the signature change to NewExprSemanticsChecker may not be what we want long term and possibly it would be better for it to receive a config object instead of individual values but I didn't think such a change was appropriate in this PR and I had no idea how to start

Overall, I did think opening a PR was a good way to get the conversation started about #609

@tdeo tdeo requested a review from rhysd May 5, 2026 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add a secrets configuration option and check much like config-variables

2 participants