Skip to content

Support for pro-builder secrets and sealing#37

Merged
alexellis merged 1 commit intomasterfrom
box_secrets
Mar 23, 2026
Merged

Support for pro-builder secrets and sealing#37
alexellis merged 1 commit intomasterfrom
box_secrets

Conversation

@alexellis
Copy link
Copy Markdown
Member

Description

Support for pro-builder secrets and sealing

  • Adds build-time secrets for pro-builder
  • Adds ability to seal secrets for use for deployment time for future features in OpenFaaS (gateway/operator-level)

Tested e2e with new pro-builder.

@reviewfn

This comment has been minimized.

* Adds build-time secrets for pro-builder
* Adds ability to seal secrets for use for deployment time for
future features in OpenFaaS (gateway/operator-level)

Tested e2e with new pro-builder.

Signed-off-by: Alex Ellis (OpenFaaS Ltd) <alexellis2@gmail.com>
@reviewfn
Copy link
Copy Markdown

reviewfn bot commented Mar 23, 2026

AI Pull Request Overview

Summary

  • Adds support for build-time secrets using authenticated public-key encryption via NaCl box
  • Introduces new seal package for encrypting secrets into YAML envelopes
  • Modifies builder to append sealed secrets to tar archives for BuildKit
  • Adds new builder methods: BuildWithSecrets and BuildWithSecretsStream
  • Updates README with usage example for encrypted build secrets
  • Adds comprehensive tests for sealing functionality
  • Updates dependencies to include golang.org/x/crypto for NaCl box

Approval rating (1-10)

9

Strong implementation with good security practices, comprehensive testing, and minimal risks. Minor documentation inconsistency noted.

Summary per file

File path Summary
README.md Adds documentation for encrypted build secrets usage
builder/build_secrets.go New file for build secrets key configuration and sealing
builder/builder.go Adds secret sealing to build process and new methods
builder/builder_test.go Adds test for BuildWithSecrets functionality
go.mod Adds golang.org/x/crypto dependency
go.sum Updates checksums for new dependencies
seal/seal.go New package implementing NaCl box encryption for secrets
seal/seal_test.go Comprehensive tests for seal package functionality

Overall Assessment

The changes introduce a secure mechanism for handling build-time secrets in OpenFaaS, using industry-standard NaCl box encryption. The implementation is well-tested and maintains backward compatibility. Risk of regressions is low as the feature is additive. Security practices are sound with proper key validation and authenticated encryption. Testing coverage is strong with e2e validation mentioned.

Detailed Review

Detailed Review

Security Considerations

  • Use of NaCl box (Curve25519 + XSalsa20-Poly1305) provides authenticated encryption, preventing tampering and ensuring confidentiality.
  • Keys are properly validated as 32-byte base64-encoded values.
  • Each secret value is encrypted independently with unique nonces, reducing risk of related-key attacks.
  • Public keys are embedded in envelopes for auditability, but private keys are never stored.

Correctness

  • appendToTar function correctly trims trailing zero blocks (tar format requirement) before appending new entries.
  • isZeroBlock helper ensures proper tar boundary detection.
  • Sealing process generates ephemeral keypairs per operation, enhancing forward secrecy.
  • Error handling is comprehensive, with specific error messages for key validation, decryption failures, and format issues.

Testing

  • Comprehensive test suite covers round-trip encryption, single key unsealing, missing keys, wrong keys, binary data, and error cases.
  • Integration test in builder_test.go verifies end-to-end flow including HMAC signing, tar modification, and unsealing.
  • Tests include edge cases like corrupted ciphertext, unsupported versions, and invalid algorithms.

Performance

  • Encryption/decryption uses efficient NaCl primitives.
  • Tar appending is O(n) on tar size, but tars are typically small for build contexts.
  • No persistent state or memory leaks in stateless functions.

Potential Issues

  • README.md example uses WithEncryptedBuildSecretsKey but code defines WithBuildSecretsKey - naming inconsistency that could confuse users.
  • BuildSecretsFileName constant uses filename com.openfaas.secrets - ensure this doesn't conflict with existing build configurations.
  • No validation that sealed secrets don't exceed reasonable size limits, potentially causing large tars.

Migration and Compatibility

  • All existing builder methods remain unchanged, ensuring backward compatibility.
  • New functionality is opt-in via new methods and options.
  • No breaking changes to existing APIs or configurations.

Code Quality

  • Consistent error wrapping with %w verb.
  • Clear function names and documentation.
  • Proper use of Go idioms (slices, maps, error handling).
  • Imports are correctly organized and minimal.

Risks

  • Dependency on golang.org/x/crypto introduces potential supply chain risks, but it's a standard, well-maintained library.
  • If build secrets are compromised, they could expose sensitive data during build time.
  • Large numbers of secrets could impact build performance due to tar operations.

Recommendations

  • Fix README naming inconsistency: change WithEncryptedBuildSecretsKey to WithBuildSecretsKey.
  • Consider adding size limits for individual secrets and total sealed data.
  • Add metrics/logging for sealing operations if monitoring is desired.
  • Document key rotation procedures for long-term maintenance.

AI agent details.

Agent processing time: 28.4s
Environment preparation time: 3.058s
Total time from webhook: 33.686s

@alexellis alexellis merged commit a779472 into master Mar 23, 2026
2 checks passed
@alexellis alexellis deleted the box_secrets branch March 23, 2026 18:22
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.

1 participant