profile-compaction: CollapseConfig CRD + projection overlay + signing/tamper detection#847
Open
matthyx wants to merge 1 commit into
Open
profile-compaction: CollapseConfig CRD + projection overlay + signing/tamper detection#847matthyx wants to merge 1 commit into
matthyx wants to merge 1 commit into
Conversation
matthyx
added
the
ai-assisted
📝 WalkthroughWalkthroughAdds a profile-signing library and CLI, runtime rule and container-profile signature verification, R1016 tamper alert emission for signed overlays, and expanded component tests, fixtures, docs, and workflow entries. ChangesProfile signing and tamper detection
Sequence Diagram(s)sequenceDiagram
participant ContainerProfileCacheImpl
participant verifyUserApplicationProfile
participant signature
participant emitTamperAlert
participant "exporters.Exporter" as Exporter
ContainerProfileCacheImpl->>verifyUserApplicationProfile: load signed overlay
verifyUserApplicationProfile->>signature: VerifyObjectAllowUntrusted(...)
signature-->>verifyUserApplicationProfile: ErrSignatureMismatch
verifyUserApplicationProfile->>emitTamperAlert: build R1016 alert
emitTamperAlert->>Exporter: SendRuleAlert(...)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~90+ minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
matthyx
force-pushed
the
profile-compaction-808
branch
from
432fef2 to
e643967
Compare
Performance Benchmark ResultsNode-Agent Resource Usage
Dedup EffectivenessNo data available. |
matthyx
force-pushed
the
profile-compaction-808
branch
2 times, most recently
from
a460877 to
eb14aa8
Compare
Performance Benchmark ResultsNode-Agent Resource Usage
Dedup EffectivenessNo data available. |
Performance Benchmark ResultsNode-Agent Resource Usage
Dedup EffectivenessNo data available. |
…naged lifecycle + signing/tamper detection Squashed and rebased onto main (resolving conflicts against the merged #806 network-wildcards and #807 exec-args work). Adds, on top of main's projection surface: - Cryptographic signing/verification of ApplicationProfile, NetworkNeighborhood, seccomp and rules profiles (pkg/signature, cosign-backed, key-based + keyless). - Tamper detection for signed user overlays: re-verifies on every ContainerProfileCache load and emits R1016 "Signed profile tampered". - Rule-signature verification in ruleswatcher. - sign-object CLI (sign / verify / generate-keypair / extract-signature). - User-defined NetworkNeighborhood overlay support + SyncChecksum overlay-identity stamping. - enableSignatureVerification config flag. Conflict resolution: kept main's tested projection design (projectField isPathSurface, ExecsByPath [][]string) and rule-binding notificationQueue; the PR's redundant reimplementation of that surface was dropped in favour of the merged #806/#807 versions. Only the net-new features above were grafted on top. go.mod: pins the official kubescape/storage v0.0.291 (carries storage #325), dropping the temporary k8sstormcenter/storage fork replace; adds sigstore (fulcio, rekor) dependencies for signing. Feature documented in docs/features/profile-signing-and-tamper-detection.md. Syncs R1016 "Signed profile tampered" into the bundled default-rules.yaml from kubescape/rulelibrary (metadata-only entry, empty ruleExpression since R1016 is code-emitted, not CEL-evaluated). Squashed from PR #808 (kubescape/node-agent). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Duck <70207455+entlein@users.noreply.github.com>
matthyx
force-pushed
the
profile-compaction-808
branch
from
eb14aa8 to
ea2d117
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (7)
cmd/sign-object/Dockerfile (1)
17-20: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winRun the container as a non-root user.
The final image runs as root.
gcr.io/distroless/static-debian13provides anonrootuser (UID 65532); switching to it reduces blast radius for a tool that handles private keys.🔒️ Proposed hardening
FROM gcr.io/distroless/static-debian13:latest COPY --from=builder /sign-object /usr/local/bin/sign-object WORKDIR /work +USER nonroot:nonroot ENTRYPOINT ["sign-object"]Note: ensure any mounted
/workvolume is writable by UID 65532 (relevant forgenerate-keypair/signoutput).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/sign-object/Dockerfile` around lines 17 - 20, The final stage of the sign-object image still runs as root; update the Dockerfile’s distroless runtime setup to use the built-in nonroot user from gcr.io/distroless/static-debian13 before the ENTRYPOINT. Make the change in the final image configuration so the container executes as UID 65532, and ensure the /work directory usage remains compatible with that user for generate-keypair/sign outputs.Source: Linters/SAST tools
pkg/signature/sign_test.go (1)
194-208: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPositive-path
GetObjectSignaturesubtest is skipped by default.The "Complete signature" case relies on
SignObjectKeyless, gated onENABLE_KEYLESS_TESTS, so the success path never runs in CI.SignObjectDisableKeyless(used throughout the rest of this PR) signs without any external dependency and would let this subtest exercise the non-error branch by default.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/signature/sign_test.go` around lines 194 - 208, The "Complete signature" test path in sign_test.go is being skipped by default because it calls SignObjectKeyless behind the ENABLE_KEYLESS_TESTS gate, so the positive GetObjectSignature branch never runs in CI. Update the setupSign path in the test table/t.Run logic to use SignObjectDisableKeyless instead of SignObjectKeyless for this subtest, keeping the env-gated skip only for cases that truly require keyless signing. This should make the success-case coverage run by default while preserving the existing test structure around GetObjectSignature and the tt.setupSign flow.pkg/rulemanager/ruleswatcher/watcher.go (1)
90-108: 🩺 Stability & Availability | 🔵 TrivialEnabling verification with unsigned rule resources silently drops all their enabled rules.
When
EnableSignatureVerification=true, any unsigned rules resource (including the bundled default rules if unsigned) is rejected byverifyRules, and every enabled rule it carries is skipped — leaving detection with potentially zero active rules. The unsigned case is only logged at Debug insideverifyRules; the sole surfaced signal is theskippedByVerificationInfo count. Consider documenting this fail-toward-empty behavior and/or emitting a Warning/metric when verification is enabled but a large fraction of rules are skipped, so operators don't unknowingly disable detection.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/rulemanager/ruleswatcher/watcher.go` around lines 90 - 108, The rules sync path in watcher.go is failing closed on unsigned resources, which can silently remove all enabled rules when EnableSignatureVerification is on. Update the RulesWatcher flow around unstructuredToRules and verifyRules to make this behavior explicit: document that unsigned resources are rejected, and add a higher-signal Warning or metric in the skippedVerificationCount path when many or all enabled rules are skipped. Keep the existing verifyRules and skippedByVerification accounting, but ensure operators get visible feedback when rule coverage drops to near zero.tests/component_test.go (1)
3370-3398: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated doc comment block for
Test_30_TamperedSignedProfiles.The function-level comment (Lines 3370-3383) is repeated verbatim immediately below (Lines 3385-3398) — likely a rebase artifact. Only the block directly preceding the
funcis the godoc comment; remove the earlier duplicate.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/component_test.go` around lines 3370 - 3398, Remove the duplicated doc comment for Test_30_TamperedSignedProfiles and keep only the single godoc block immediately preceding the test function. Clean up the repeated comment text in the component_test.go test section so the Test_30_TamperedSignedProfiles declaration has just one unique documentation block attached to it.pkg/objectcache/containerprofilecache/tamper_alert_test.go (1)
350-351: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the stale trailing comment. The comment describes a
cfgRefconfig shim, but no such type exists in the file — the strict-mode tests constructconfig.Configdirectly (lines 312, 342). This dangling comment is misleading leftover.♻️ Proposed cleanup
- -// cfgRef is a minimal config shim for the strict-mode tests. Mirrors the -// concrete config.Config struct shape only in the field the verifier reads.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/objectcache/containerprofilecache/tamper_alert_test.go` around lines 350 - 351, Remove the stale trailing comment in tamper_alert_test.go that refers to a cfgRef config shim, since the strict-mode tests use config.Config directly and no cfgRef type exists. Locate the leftover comment near the strict-mode test setup around the existing config.Config usage, and delete only that misleading note without changing the test logic.pkg/signature/cosign_adapter.go (2)
232-234: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove dead
simulateKeyless. It is unused and only returns a deprecation error.♻️ Proposed removal
-func (c *CosignAdapter) simulateKeyless(data []byte) (*Signature, error) { - return nil, fmt.Errorf("simulateKeyless is deprecated, use real keyless signing") -} -🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/signature/cosign_adapter.go` around lines 232 - 234, The simulateKeyless method on CosignAdapter is dead code and should be removed entirely rather than kept as a deprecation stub. Delete the unused simulateKeyless function from cosign_adapter.go, and make sure no remaining references in CosignAdapter or related signature flow still depend on it; if any callers exist, update them to use the real keyless signing path instead.Source: Linters/SAST tools
37-45: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueBlank
var _ =references mask unused imports. Several of these (cosign.Signature,bundle.RekorBundle,client.Rekor,models.LogEntry) exist only to keep therekor/cosignimports compiling while their real usage is commented-out placeholder code. Once the Rekor flow is implemented (or dropped), remove these blanks and the corresponding imports to keep the dependency surface honest.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/signature/cosign_adapter.go` around lines 37 - 45, The blank var _ references in cosign_adapter.go are only keeping placeholder Rekor/Cosign imports alive and should be removed once real usage is added or the flow is dropped. Update the cosign_adapter package by cleaning out the unused symbols like cosign.Signature, bundle.RekorBundle, client.Rekor, and models.LogEntry, and then remove any imports that are no longer needed so the file reflects actual runtime dependencies.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/sign-object/main.go`:
- Line 549: The help text in sign-object’s CLI output still points to a missing
docs target, so update the reference in main.go where the usage/help string is
built to use docs/features/profile-signing-and-tamper-detection.md instead of
the broken signing README path. Make sure the updated text appears in the same
help-message/usage block that prints “For more information” so sign-object
--help shows a valid link.
In `@go.mod`:
- Around line 51-52: The dependency on github.com/sigstore/rekor is still pinned
to the vulnerable 1.5.0 release; update the rekor entry in go.mod to 1.5.2 or
newer while leaving github.com/sigstore/fulcio as-is. Make sure the module
version change is reflected wherever the rekor dependency is declared so the
project no longer resolves to the affected release.
In `@pkg/signature/cosign_adapter_test.go`:
- Around line 128-142: The “Certificate is CA” subtest in cosign_adapter_test.go
is not actually reaching the CA-check in VerifyData because it feeds raw DER
into Signature.Certificate and ignores CreateCertificate errors, so the failure
can come from the empty-cert or public-key path instead. Update this test to
generate a valid certificate with x509.CreateCertificate, ensure it has required
fields like SerialNumber so creation succeeds, PEM-encode the resulting cert
before assigning it to Signature.Certificate, and then assert the specific
CA-rejection error from adapter.VerifyData so the subtest exercises the intended
cert.IsCA branch.
In `@pkg/signature/cosign_adapter.go`:
- Around line 449-459: In cosign_adapter.go, the strict verification logic in
the Rekor/CT evidence check is rejecting ambient OIDC keyless signatures because
signKeyless does not populate sig.RekorBundle, while only the hard-coded
interactive issuer is exempted. Update the verification path around the
sig.RekorBundle / sig.Issuer checks so ambient workload-identity issuers are
either given Rekor evidence during signing or explicitly treated as valid in
strict mode, and keep the policy consistent with signKeyless and the strict
verification branch.
---
Nitpick comments:
In `@cmd/sign-object/Dockerfile`:
- Around line 17-20: The final stage of the sign-object image still runs as
root; update the Dockerfile’s distroless runtime setup to use the built-in
nonroot user from gcr.io/distroless/static-debian13 before the ENTRYPOINT. Make
the change in the final image configuration so the container executes as UID
65532, and ensure the /work directory usage remains compatible with that user
for generate-keypair/sign outputs.
In `@pkg/objectcache/containerprofilecache/tamper_alert_test.go`:
- Around line 350-351: Remove the stale trailing comment in tamper_alert_test.go
that refers to a cfgRef config shim, since the strict-mode tests use
config.Config directly and no cfgRef type exists. Locate the leftover comment
near the strict-mode test setup around the existing config.Config usage, and
delete only that misleading note without changing the test logic.
In `@pkg/rulemanager/ruleswatcher/watcher.go`:
- Around line 90-108: The rules sync path in watcher.go is failing closed on
unsigned resources, which can silently remove all enabled rules when
EnableSignatureVerification is on. Update the RulesWatcher flow around
unstructuredToRules and verifyRules to make this behavior explicit: document
that unsigned resources are rejected, and add a higher-signal Warning or metric
in the skippedVerificationCount path when many or all enabled rules are skipped.
Keep the existing verifyRules and skippedByVerification accounting, but ensure
operators get visible feedback when rule coverage drops to near zero.
In `@pkg/signature/cosign_adapter.go`:
- Around line 232-234: The simulateKeyless method on CosignAdapter is dead code
and should be removed entirely rather than kept as a deprecation stub. Delete
the unused simulateKeyless function from cosign_adapter.go, and make sure no
remaining references in CosignAdapter or related signature flow still depend on
it; if any callers exist, update them to use the real keyless signing path
instead.
- Around line 37-45: The blank var _ references in cosign_adapter.go are only
keeping placeholder Rekor/Cosign imports alive and should be removed once real
usage is added or the flow is dropped. Update the cosign_adapter package by
cleaning out the unused symbols like cosign.Signature, bundle.RekorBundle,
client.Rekor, and models.LogEntry, and then remove any imports that are no
longer needed so the file reflects actual runtime dependencies.
In `@pkg/signature/sign_test.go`:
- Around line 194-208: The "Complete signature" test path in sign_test.go is
being skipped by default because it calls SignObjectKeyless behind the
ENABLE_KEYLESS_TESTS gate, so the positive GetObjectSignature branch never runs
in CI. Update the setupSign path in the test table/t.Run logic to use
SignObjectDisableKeyless instead of SignObjectKeyless for this subtest, keeping
the env-gated skip only for cases that truly require keyless signing. This
should make the success-case coverage run by default while preserving the
existing test structure around GetObjectSignature and the tt.setupSign flow.
In `@tests/component_test.go`:
- Around line 3370-3398: Remove the duplicated doc comment for
Test_30_TamperedSignedProfiles and keep only the single godoc block immediately
preceding the test function. Clean up the repeated comment text in the
component_test.go test section so the Test_30_TamperedSignedProfiles declaration
has just one unique documentation block attached to it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ce71f69f-fb9b-495d-b391-f54e08cfec6a
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (44)
.github/workflows/component-tests.yamlcmd/main.gocmd/sign-object/Dockerfilecmd/sign-object/main.godocs/features/profile-signing-and-tamper-detection.mdgo.modpkg/config/config.gopkg/containerprofilemanager/v1/lifecycle.gopkg/exporters/alert_manager.gopkg/objectcache/containerprofilecache/containerprofilecache.gopkg/objectcache/containerprofilecache/projection.gopkg/objectcache/containerprofilecache/tamper_alert.gopkg/objectcache/containerprofilecache/tamper_alert_test.gopkg/objectcache/containerprofilecache/test32_projection_test.gopkg/objectcache/shared_container_data.gopkg/rulemanager/cel/libraries/applicationprofile/ap.gopkg/rulemanager/cel/libraries/cache/function_cache.gopkg/rulemanager/ruleswatcher/watcher.gopkg/rulemanager/ruleswatcher/watcher_signature_test.gopkg/signature/annotations.gopkg/signature/cluster_flow_test.gopkg/signature/cluster_scenario_test.gopkg/signature/cosign_adapter.gopkg/signature/cosign_adapter_test.gopkg/signature/interface.gopkg/signature/profiles/adapter_test.gopkg/signature/profiles/applicationprofile_adapter.gopkg/signature/profiles/empty_typemeta_test.gopkg/signature/profiles/networkneighborhood_adapter.gopkg/signature/profiles/networkneighborhood_adapter_test.gopkg/signature/profiles/rules_adapter.gopkg/signature/profiles/rules_adapter_test.gopkg/signature/profiles/seccompprofile_adapter.gopkg/signature/sign.gopkg/signature/sign_test.gopkg/signature/signer.gopkg/signature/verifier.gopkg/signature/verify.gopkg/signature/verify_test.gotests/chart/templates/node-agent/default-rules.yamltests/component_test.gotests/resources/aplint_test.gotests/resources/curl-signed-deployment.yamltests/resources/known-application-profile.yaml
| # Extract signature information | ||
| sign-object extract-signature --file signed-object.yaml | ||
|
|
||
| For more information, see: docs/signing/README.md`) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -t f 'README.md' docs/signing 2>/dev/null || echo "docs/signing/README.md not found"
fd -t f 'profile-signing-and-tamper-detection.md' docsRepository: kubescape/node-agent
Length of output: 245
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the nearby help text in context
sed -n '530,560p' cmd/sign-object/main.go
# Check whether the old docs path is referenced anywhere else
rg -n "docs/signing/README\.md|profile-signing-and-tamper-detection\.md" .Repository: kubescape/node-agent
Length of output: 910
Fix the help-text docs link
docs/signing/README.md is missing; update this to docs/features/profile-signing-and-tamper-detection.md so sign-object --help doesn’t advertise a broken reference.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmd/sign-object/main.go` at line 549, The help text in sign-object’s CLI
output still points to a missing docs target, so update the reference in main.go
where the usage/help string is built to use
docs/features/profile-signing-and-tamper-detection.md instead of the broken
signing README path. Make sure the updated text appears in the same
help-message/usage block that prints “For more information” so sign-object
--help shows a valid link.
Comment on lines
+51
to
+52
| github.com/sigstore/fulcio v1.8.5 | ||
| github.com/sigstore/rekor v1.5.0 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
for pkg in "github.com/sigstore/cosign" "github.com/sigstore/fulcio" "github.com/sigstore/rekor" "github.com/sigstore/sigstore" "github.com/golang-jwt/jwt"; do
echo "== $pkg =="
gh api graphql -f query='
query($q:String!){ securityVulnerabilities(first:5, ecosystem:GO, package:$q){ nodes{ advisory{summary severity} vulnerableVersionRange firstPatchedVersion{identifier} } } }' -f q="$pkg"
doneRepository: kubescape/node-agent
Length of output: 3495
🏁 Script executed:
#!/bin/bash
set -euo pipefail
nl -ba go.mod | sed -n '45,60p'Repository: kubescape/node-agent
Length of output: 198
Bump github.com/sigstore/rekor off the vulnerable 1.5.0 release github.com/sigstore/rekor v1.5.0 is still in the high-severity OOM advisory range (< 1.5.2); update it to 1.5.2 or newer. github.com/sigstore/fulcio v1.8.5 is past the listed fix.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@go.mod` around lines 51 - 52, The dependency on github.com/sigstore/rekor is
still pinned to the vulnerable 1.5.0 release; update the rekor entry in go.mod
to 1.5.2 or newer while leaving github.com/sigstore/fulcio as-is. Make sure the
module version change is reflected wherever the rekor dependency is declared so
the project no longer resolves to the affected release.
Comment on lines
+128
to
+142
| t.Run("Certificate is CA", func(t *testing.T) { | ||
| // Create a CA certificate | ||
| template := x509.Certificate{ | ||
| IsCA: true, | ||
| } | ||
| certDER, _ := x509.CreateCertificate(rand.Reader, &template, &template, &privKey.PublicKey, privKey) | ||
| sig := &Signature{ | ||
| Signature: []byte("sig"), | ||
| Certificate: certDER, | ||
| } | ||
| err := adapter.VerifyData(data, sig, false) | ||
| if err == nil { | ||
| t.Error("Expected error for CA certificate, got nil") | ||
| } | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
This subtest never exercises the CA-rejection path.
Signature.Certificate is assigned raw DER bytes, but VerifyData runs pem.Decode first; on non-PEM input the block is nil and execution falls into the public-key branch, returning "untrusted public key rejected" rather than the cert.IsCA rejection. The discarded CreateCertificate error compounds this — without a SerialNumber the call likely fails, leaving certDER empty and triggering the "Certificate is empty" guard instead. The assertion passes for the wrong reason.
🧪 Proposed fix: PEM-encode a valid cert and assert the error
t.Run("Certificate is CA", func(t *testing.T) {
- // Create a CA certificate
- template := x509.Certificate{
- IsCA: true,
- }
- certDER, _ := x509.CreateCertificate(rand.Reader, &template, &template, &privKey.PublicKey, privKey)
- sig := &Signature{
- Signature: []byte("sig"),
- Certificate: certDER,
- }
+ // Create a CA certificate
+ template := x509.Certificate{
+ SerialNumber: big.NewInt(1),
+ IsCA: true,
+ }
+ certDER, err := x509.CreateCertificate(rand.Reader, &template, &template, &privKey.PublicKey, privKey)
+ if err != nil {
+ t.Fatalf("failed to create certificate: %v", err)
+ }
+ certPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: certDER})
+ sig := &Signature{
+ Signature: []byte("sig"),
+ Certificate: certPEM,
+ }
err := adapter.VerifyData(data, sig, false)
if err == nil {
t.Error("Expected error for CA certificate, got nil")
}
})Requires adding "crypto/rand", "encoding/pem", and "math/big" imports.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/signature/cosign_adapter_test.go` around lines 128 - 142, The
“Certificate is CA” subtest in cosign_adapter_test.go is not actually reaching
the CA-check in VerifyData because it feeds raw DER into Signature.Certificate
and ignores CreateCertificate errors, so the failure can come from the
empty-cert or public-key path instead. Update this test to generate a valid
certificate with x509.CreateCertificate, ensure it has required fields like
SerialNumber so creation succeeds, PEM-encode the resulting cert before
assigning it to Signature.Certificate, and then assert the specific CA-rejection
error from adapter.VerifyData so the subtest exercises the intended cert.IsCA
branch.
Comment on lines
+449
to
+459
| // Validate Rekor/CT evidence if Rekor bundle is present | ||
| if len(sig.RekorBundle) > 0 { | ||
| // In a full implementation, we would use cosign.VerifyBundle | ||
| // for now we acknowledge its presence for strict verification | ||
| } else if sig.Issuer != "local" && sig.Issuer != "" { | ||
| // For non-local certificates, we expect a Rekor bundle in strict mode | ||
| // But we'll allow it if we are in interactive mode (where Rekor might not be used) | ||
| if sig.Issuer != "https://oauth2.sigstore.dev/auth" { | ||
| return fmt.Errorf("strict verification failed: missing Rekor bundle for certificate from %q", sig.Issuer) | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== file outline ==\n'
ast-grep outline pkg/signature/cosign_adapter.go --view expanded || true
printf '\n== relevant slices ==\n'
sed -n '180,240p' pkg/signature/cosign_adapter.go
printf '\n---\n'
sed -n '430,480p' pkg/signature/cosign_adapter.go
printf '\n== search RekorBundle assignments/usages ==\n'
rg -n "RekorBundle|issuer|allowUntrusted|strict verification" pkg/signature -SRepository: kubescape/node-agent
Length of output: 10052
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== verification tail ==\n'
sed -n '480,510p' pkg/signature/cosign_adapter.go
printf '\n== keyless signing header/body ==\n'
sed -n '130,230p' pkg/signature/cosign_adapter.go
printf '\n== strict verification tests/search ==\n'
rg -n "strict verification|missing Rekor bundle|allowUntrusted|keyless signature missing issuer or identity|useKeyless" pkg/signature -SRepository: kubescape/node-agent
Length of output: 6423
Keyless signatures from ambient OIDC fail strict verification. signKeyless never sets sig.RekorBundle, so strict mode rejects workload-identity issuers such as GitHub Actions; only the hard-coded interactive issuer (https://oauth2.sigstore.dev/auth) bypasses that check. Either upload Rekor evidence or document that strict mode only works for the interactive keyless flow.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/signature/cosign_adapter.go` around lines 449 - 459, In
cosign_adapter.go, the strict verification logic in the Rekor/CT evidence check
is rejecting ambient OIDC keyless signatures because signKeyless does not
populate sig.RekorBundle, while only the hard-coded interactive issuer is
exempted. Update the verification path around the sig.RekorBundle / sig.Issuer
checks so ambient workload-identity issuers are either given Rekor evidence
during signing or explicitly treated as valid in strict mode, and keep the
policy consistent with signKeyless and the strict verification branch.
Performance Benchmark ResultsNode-Agent Resource Usage
Dedup EffectivenessNo data available. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Same-repo mirror of #808 (which is from the
k8sstormcenterfork) so the gated Node Agent Component Tests and build-and-push-image (pull_request) workflows run with repo secrets.Single squashed commit, rebased on
main, conflicts resolved.Overview
Adds cryptographic profile signing & tamper detection plus the CollapseConfig projection-overlay / user-managed lifecycle work:
pkg/signature(cosign-backed sign/verify; key-based + keyless) + per-kind adapters (AP / NN / seccomp / rules)ruleswatchersign-objectCLI (sign/verify/generate-keypair/extract-signature)enableSignatureVerificationconfig flagConflict resolution
This PR predates the merged #806 (network wildcards) and #807 (exec-args) and independently reimplemented the projection surface. Per maintainer decision, main's tested design was kept (
projectField(isPathSurface),ExecsByPath [][]string, rule-bindingnotificationQueue); the PR's redundant reimplementation was dropped and only its net-new features grafted on top.Storage dependency
Backed by kubescape/storage #325.
go.modpins the officialkubescape/storage v0.0.291(drops thek8sstormcenter/storagefork replace) and adds sigstore (fulcio, rekor) deps.Verification (local)
go build ./...✓ ·go vet ./...✓ · unit tests green forpkg/signature/...,containerprofilecache(incl.test32_projection_test,tamper_alert_test),rulebindingmanager/cache,ruleswatcher, andnetworkneighborhood(confirms #806 intact).Mirrors #808.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation