Repository navigation
Conversation
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
📝 Walkthrough
Merge Risk: 🔵 Low · up to
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ce94ba306
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
🔇 Additional comments: 4
plugins/out_bigquery/bigquery.c: 1
254-255: LGTM!plugins/out_chronicle/chronicle.c: 1
219-220: LGTM!plugins/out_stackdriver/stackdriver.c: 1
426-427: LGTM!tests/integration/scenarios/out_stackdriver/tests/test_assertion_logging.py-86-86: 1
86-86: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
⚠️ Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.The JWT regex can match unrelated log text and cause flaky failures.
The pattern
[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+\.[A-Za-z0-9_-]{20,}matches any dotted token with a long final segment. Examples are hostnames, file paths, and version strings. Debug and trace logs contain such text. The test can then fail without any assertion leak.Anchor the pattern on the JWT header prefix. Every JWT built by these plugins starts with the base64 of
{"alg": "RS256", "typ": "JWT"}. That prefix iseyJhbGciOiAiUlMyNTYi.Proposed fix
- assert not re.search(r"[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+\.[A-Za-z0-9_-]{20,}", logs) + assert not re.search(r"eyJhbGciOiAiUlMyNTYi[A-Za-z0-9_-]*\.[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+", logs)
ℹ️ Review info
⚙️ Run configuration
- Configuration used: defaults
- Review profile: CHILL
- Plan: Advanced
- Run ID:
6df8d5cc-b8b3-4dc5-88c5-ca7900068651
📒 Files selected for processing: 4
plugins/out_bigquery/bigquery.cplugins/out_chronicle/chronicle.cplugins/out_stackdriver/stackdriver.ctests/integration/scenarios/out_stackdriver/tests/test_assertion_logging.py
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.
Mitigated all three outputs: Stackdriver, BigQuery, and Chronicle now log only
JWT assertion generated, without credential contents. Added six regression cases covering debug and trace logging.Verification:
Operation not permitted. Valgrind verification therefore remains incomplete.Can you approve local-socket test execution when approval is available? Automatic approval review failed twice because its model was at capacity, leaving these checks blocked.
Enter
[N/A]in the box, if an item is not applicable to your change.Testing
Before we can approve your change; please submit the following in a comment:
If this is a change to packaging of containers or native binaries then please confirm it works for all targets.
ok-package-testlabel to test for all targets (requires maintainer to do).Documentation
Backporting
Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.
Summary by CodeRabbit