out_oracle_log_analytics: Implement IMDS authentication, chunking#10990
out_oracle_log_analytics: Implement IMDS authentication, chunking#10990rghouzra wants to merge 4 commits intofluent:masterfrom
Conversation
WalkthroughAdds a helper module for region/realm/timezone mapping, integrates IMDS + federation instance‑principal authentication with session keypair and token exchange/refresh, implements OCI request signing, payload batching/chunking and optional local dumping, updates build/tests, and exposes helper/config APIs. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant FB as Fluent Bit
participant CONF as oci_logan_conf
participant HELP as oci_logan_helper
participant PL as oci_logan
participant IMDS as OCI IMDS
participant FED as OCI Federation
participant LA as Oracle Log Analytics
rect rgb(250,250,255)
note over FB,CONF: Initialization & auth
FB->>CONF: flb_oci_logan_conf_create()
alt auth_type == instance_principal
CONF->>IMDS: fetch region, certs, keys
IMDS-->>CONF: region/certs/keys
CONF->>CONF: generate session RSA keypair
CONF->>CONF: create_federation_payload()
CONF->>FED: sign_and_send_federation_request(payload)
FED-->>CONF: federation token (JWT)
CONF->>CONF: decode JWT, set token expiry
else auth_type == config_file
CONF->>CONF: load credentials from config file
end
CONF-->>FB: initialized ctx
end
rect rgb(245,255,245)
note over FB,PL: Flush / Send logs
FB->>PL: cb_flush(chunk)
PL->>HELP: determine_realm_from_region / is_valid_timezone
PL->>PL: token_needs_refresh?
alt needs_refresh
PL->>CONF: sign_and_send_federation_request()
CONF-->>PL: refreshed token
end
PL->>PL: estimate payload size -> batch decisions
loop per batch
PL->>PL: calculate_content_sha256_b64(payload)
PL->>PL: sign_oci_request_with_security_token_for_logging(...)
PL->>LA: POST signed request
alt success
LA-->>PL: 2xx OK
PL->>PL: optional dump_payload_to_file()
else error
LA-->>PL: 4xx/5xx
PL->>PL: retry/diagnose
end
end
PL-->>FB: flush result
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/out_oracle_log_analytics/oci_logan_conf.c (1)
1576-1616: Free newly allocated IMDS resources
flb_oci_logan_conf_destroystill only releases the legacy fields. Everything added for IMDS/federation—ctx->imds.region,leaf_cert,leaf_key,intermediate_cert,tenancy_ocid,fingerprint,session_pubkey,session_privkey,ctx->session_key_pair,ctx->security_token.token, plusauth_type,payload_files_location, etc.— leaks on teardown. Please destroy each SDS/heap allocation andEVP_PKEY_free(ctx->session_key_pair)before freeingctx.
🧹 Nitpick comments (1)
plugins/out_oracle_log_analytics/oci_logan_helper.c (1)
493-543: Remove stderr debug spamAll the
fprintf(stderr, ...)calls inside the timezone helpers will spam hundreds of lines per invocation and bypass Fluent Bit’s logging facilities. Please drop them or switch toflb_plg_debug/flb_plg_infoguarded by log level checks so normal runs stay clean.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
plugins/out_oracle_log_analytics/CMakeLists.txt(1 hunks)plugins/out_oracle_log_analytics/oci_logan.c(33 hunks)plugins/out_oracle_log_analytics/oci_logan.h(5 hunks)plugins/out_oracle_log_analytics/oci_logan_conf.c(13 hunks)plugins/out_oracle_log_analytics/oci_logan_conf.h(1 hunks)plugins/out_oracle_log_analytics/oci_logan_helper.c(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
plugins/out_oracle_log_analytics/oci_logan.h (1)
plugins/out_oracle_log_analytics/oci_logan_helper.c (4)
is_valid_timezone(530-544)get_domain_suffix_for_realm(562-575)determine_realm_from_region(547-559)long_region_name(579-587)
plugins/out_oracle_log_analytics/oci_logan_conf.h (1)
plugins/out_oracle_log_analytics/oci_logan_conf.c (2)
create_federation_payload(793-830)sign_and_send_federation_request(1165-1311)
plugins/out_oracle_log_analytics/oci_logan.c (8)
src/flb_sds.c (6)
flb_sds_create_size(92-95)flb_sds_cat_safe(204-214)flb_sds_destroy(389-399)flb_sds_create(78-90)flb_sds_printf(336-387)flb_sds_create_len(58-76)src/flb_http_client.c (5)
flb_http_add_header(963-995)flb_http_client(814-859)flb_http_client_destroy(1688-1695)flb_http_allow_duplicated_headers(99-107)flb_http_buffer_size(872-883)plugins/out_oracle_log_analytics/oci_logan_conf.c (2)
create_federation_payload(793-830)sign_and_send_federation_request(1165-1311)include/fluent-bit/flb_mem.h (1)
flb_free(126-128)src/flb_upstream.c (1)
flb_upstream_conn_get(711-844)src/flb_log_event_decoder.c (3)
flb_log_event_decoder_init(99-116)flb_log_event_decoder_next(310-406)flb_log_event_decoder_destroy(147-179)src/flb_pack.c (1)
flb_msgpack_raw_to_json_sds(1026-1085)src/flb_mp.c (1)
flb_mp_count(43-46)
plugins/out_oracle_log_analytics/oci_logan_helper.c (1)
src/flb_hash_table.c (4)
flb_hash_table_create(99-137)flb_hash_table_add(401-494)flb_hash_table_destroy(197-215)flb_hash_table_get(496-522)
plugins/out_oracle_log_analytics/oci_logan_conf.c (7)
src/flb_http_client.c (4)
flb_http_client(814-859)flb_http_add_header(963-995)flb_http_do(1572-1632)flb_http_client_destroy(1688-1695)src/flb_sds.c (5)
flb_sds_create_len(58-76)flb_sds_create_size(92-95)flb_sds_create(78-90)flb_sds_destroy(389-399)flb_sds_printf(336-387)plugins/out_oracle_log_analytics/oci_logan_helper.c (3)
determine_realm_from_region(547-559)get_domain_suffix_for_realm(562-575)long_region_name(579-587)include/fluent-bit/flb_mem.h (2)
flb_calloc(84-96)flb_free(126-128)src/flb_upstream.c (4)
flb_upstream_create(290-363)flb_upstream_conn_get(711-844)flb_upstream_conn_release(862-947)flb_upstream_destroy(656-698)src/flb_output.c (1)
flb_output_net_default(1059-1069)src/flb_utils.c (1)
flb_utils_url_split(1441-1534)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (30)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_ARROW=On, 3.31.6, gcc, g++)
- GitHub Check: PR - fuzzing test
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
- GitHub Check: pr-compile-centos-7
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-without-cxx (3.31.6)
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
plugins/out_oracle_log_analytics/oci_logan_helper.c (2)
462-466: Previous issue has been resolved.The timezone hash value storage issue flagged in the previous review has been fixed. The code now correctly stores the full string including the null terminator using
sizeof("1").
488-490: CRITICAL: Uninitialized variables and unchecked hash lookup.This critical issue flagged in the previous review has NOT been fixed. The variables
out_bufandout_sizeare not initialized, and the return value fromflb_hash_table_getis not checked before dereferencingout_buf. This leads to undefined behavior and potential crashes when a timezone is not found in the hash table.Apply the fix from the previous review:
- void *out_buf; - size_t out_size; + void *out_buf = NULL; + size_t out_size = 0; + int ht_ret; @@ - flb_hash_table_get(oci_timezone_hash, lower_tz, strlen(lower_tz), - &out_buf, &out_size); + ht_ret = flb_hash_table_get(oci_timezone_hash, lower_tz, + strlen(lower_tz), &out_buf, &out_size); + if (ht_ret != 0) { + free(lower_tz); + return 0; + } @@ - fprintf(stderr, "is_oci_supported_timezone::out_buf->%s\n", - (char *) out_buf); - fflush(stderr); - free(lower_tz); - return ((out_buf != NULL ? (!strcmp(out_buf, "1") ? 1 : 0) : 0)); + free(lower_tz); + return (out_buf != NULL && strcmp((char *) out_buf, "1") == 0) ? 1 : 0;Also applies to: 519-527
🧹 Nitpick comments (1)
plugins/out_oracle_log_analytics/oci_logan_helper.c (1)
445-476: Consider using consistent error codes.The initialization function returns different negative error codes (-2, -3), but the callers only check for non-zero. Consider documenting these error codes or simplifying to return 0 for success and -1 for any failure.
Example:
- return -2; + return -1; /* hash table creation failed */ @@ - return -3; + return -1; /* hash table population failed */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
plugins/out_oracle_log_analytics/oci_logan_helper.c(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
plugins/out_oracle_log_analytics/oci_logan_helper.c (1)
src/flb_hash_table.c (4)
flb_hash_table_create(99-137)flb_hash_table_add(401-494)flb_hash_table_destroy(197-215)flb_hash_table_get(496-522)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (30)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-22.04, clang-12)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_ARROW=On, 3.31.6, gcc, g++)
- GitHub Check: pr-compile-without-cxx (3.31.6)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-centos-7
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: PR - fuzzing test
🔇 Additional comments (2)
plugins/out_oracle_log_analytics/oci_logan_helper.c (2)
547-575: LGTM: Realm and region helper functions are well-implemented.Both
determine_realm_from_regionandget_domain_suffix_for_realmproperly handle NULL inputs with sensible defaults ("oc1" and "oraclecloud.com" respectively) and include fallback values when lookups fail. The linear search approach is appropriate for these small static lookup tables.
478-484: LGTM: Cleanup function is correctly implemented.The cleanup function properly checks for NULL before destroying and sets the pointer to NULL afterward, preventing double-free issues.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/out_oracle_log_analytics/oci_logan_conf.c (1)
1654-1689: Release IMDS/session resources inconf_destroy().
flb_oci_logan_conf_destroy()now owns several allocations (IMDS cert/key SDS, session key pair, security token, payload dump path,auth_type, etc.) but none of them are released here, so every plugin shutdown leaks OpenSSL keys, SDS buffers, and heap strings. Please free the new fields (useflb_sds_destroyfor SDS,flb_freefor plainmalloc/flb_malloc, andEVP_PKEY_freeforsession_key_pair) before freeingctx.if (ctx->u) { flb_upstream_destroy(ctx->u); } + if (ctx->session_key_pair) { + EVP_PKEY_free(ctx->session_key_pair); + } + if (ctx->security_token.token) { + flb_sds_destroy(ctx->security_token.token); + } + if (ctx->imds.region) { + flb_sds_destroy(ctx->imds.region); + } + if (ctx->imds.leaf_cert) { + flb_free(ctx->imds.leaf_cert); + } + if (ctx->imds.leaf_key) { + flb_sds_destroy(ctx->imds.leaf_key); + } + if (ctx->imds.intermediate_cert) { + flb_sds_destroy(ctx->imds.intermediate_cert); + } + if (ctx->imds.fingerprint) { + flb_sds_destroy(ctx->imds.fingerprint); + } + if (ctx->imds.session_pubkey) { + flb_free(ctx->imds.session_pubkey); + } + if (ctx->imds.session_privkey) { + flb_free(ctx->imds.session_privkey); + } + if (ctx->auth_type) { + flb_free(ctx->auth_type); + } + if (ctx->payload_files_location) { + flb_free(ctx->payload_files_location); + } + if (ctx->domain_suffix) { + flb_free(ctx->domain_suffix); + } + if (ctx->oci_la_timezone) { flb_sds_destroy(ctx->oci_la_timezone); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
plugins/out_oracle_log_analytics/oci_logan.h(5 hunks)plugins/out_oracle_log_analytics/oci_logan_conf.c(13 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
plugins/out_oracle_log_analytics/oci_logan.h (1)
plugins/out_oracle_log_analytics/oci_logan_helper.c (4)
is_valid_timezone(520-530)get_domain_suffix_for_realm(548-561)determine_realm_from_region(533-545)long_region_name(565-576)
plugins/out_oracle_log_analytics/oci_logan_conf.c (6)
src/flb_http_client.c (4)
flb_http_client(814-859)flb_http_add_header(963-995)flb_http_do(1572-1632)flb_http_client_destroy(1688-1695)src/flb_sds.c (6)
flb_sds_create_len(58-76)flb_sds_create_size(92-95)flb_sds_snprintf(405-428)flb_sds_create(78-90)flb_sds_destroy(389-399)flb_sds_printf(336-387)plugins/out_oracle_log_analytics/oci_logan_helper.c (3)
determine_realm_from_region(533-545)get_domain_suffix_for_realm(548-561)long_region_name(565-576)src/flb_upstream.c (4)
flb_upstream_create(290-363)flb_upstream_conn_get(711-844)flb_upstream_conn_release(862-947)flb_upstream_destroy(656-698)src/flb_output.c (1)
flb_output_net_default(1059-1069)src/flb_utils.c (1)
flb_utils_url_split(1441-1534)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (29)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-without-cxx (3.31.6)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-centos-7
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_ARROW=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
- GitHub Check: PR - fuzzing test
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
rghouzra
changed the title
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
plugins/out_oracle_log_analytics/oci_logan.h (1)
115-117: Use application/json for LA JSON uploadsFor UploadLogEventsFile with JSON payloads, Content-Type should be application/json, not application/octet-stream.
-#define FLB_OCI_HEADER_CONTENT_TYPE_VAL "application/octet-stream" +#define FLB_OCI_HEADER_CONTENT_TYPE_VAL "application/json"Adjust any binary (gzip/zip) branches separately if needed.
plugins/out_oracle_log_analytics/oci_logan_conf.c (1)
1712-1754: Free IMDS fields, token, and session key in conf_destroySeveral allocations are never released (IMDS strings, token, session keys). Add proper cleanup.
int flb_oci_logan_conf_destroy(struct flb_oci_logan *ctx) { if (ctx == NULL) { return 0; } @@ if (ctx->u) { flb_upstream_destroy(ctx->u); } if (ctx->oci_la_timezone) { flb_sds_destroy(ctx->oci_la_timezone); } + + /* IMDS / federation allocations */ + if (ctx->imds.region) flb_sds_destroy(ctx->imds.region); + if (ctx->imds.leaf_cert) flb_sds_destroy(ctx->imds.leaf_cert); + if (ctx->imds.leaf_key) flb_sds_destroy(ctx->imds.leaf_key); + if (ctx->imds.intermediate_cert) flb_sds_destroy(ctx->imds.intermediate_cert); + if (ctx->imds.fingerprint) flb_sds_destroy(ctx->imds.fingerprint); + if (ctx->imds.session_pubkey) flb_free(ctx->imds.session_pubkey); + if (ctx->imds.session_privkey) flb_free(ctx->imds.session_privkey); + if (ctx->session_key_pair) EVP_PKEY_free(ctx->session_key_pair); + if (ctx->security_token.token) flb_sds_destroy(ctx->security_token.token); metadata_fields_destroy(ctx); flb_free(ctx); return 0; }
♻️ Duplicate comments (10)
plugins/out_oracle_log_analytics/oci_logan_conf.c (6)
157-161: Free ‘content’ SDS on empty file to avoid leak- if (content == NULL || flb_sds_len(content) == 0) { - return -1; - } + if (content == NULL || flb_sds_len(content) == 0) { + if (content) { flb_sds_destroy(content); } + return -1; + }
424-439: Leak: free temporary region buffer before returnflb_sds_t lregion = flb_sds_create(region_value); if (!lregion) { free(region); return NULL; } - return lregion; + free(region); + return lregion;
631-651: Free key_resp after extracting PEM; convert leaf_cert to SDSkey_resp (SDS) is leaked on success. Also normalize leaf_cert to SDS to match struct type and simplify destruction.
- ctx->imds.region = clean_region_resp; - ctx->imds.leaf_cert = clean_cert_resp; + ctx->imds.region = clean_region_resp; /* SDS */ + ctx->imds.leaf_cert = flb_sds_create(clean_cert_resp); /* to SDS */ + flb_free(clean_cert_resp); ctx->imds.intermediate_cert = int_cert_resp; @@ size_t pem_len = (pem_end - pem_start) + strlen("-----END RSA PRIVATE KEY-----") + 1; ctx->imds.leaf_key = flb_sds_create_len(pem_start, pem_len); + flb_sds_destroy(key_resp);
671-693: Error path must release partially assigned IMDS fieldsJust NULLing pointers leaks memory when earlier steps succeeded.
- ctx->imds.fingerprint = NULL; - ctx->imds.intermediate_cert = NULL; - ctx->imds.leaf_cert = NULL; - ctx->imds.leaf_key = NULL; - ctx->imds.region = NULL; + if (ctx->imds.fingerprint) { flb_sds_destroy(ctx->imds.fingerprint); ctx->imds.fingerprint = NULL; } + if (ctx->imds.intermediate_cert) { flb_sds_destroy(ctx->imds.intermediate_cert); ctx->imds.intermediate_cert = NULL; } + if (ctx->imds.leaf_cert) { flb_sds_destroy(ctx->imds.leaf_cert); ctx->imds.leaf_cert = NULL; } + if (ctx->imds.leaf_key) { flb_sds_destroy(ctx->imds.leaf_key); ctx->imds.leaf_key = NULL; } + if (ctx->imds.region) { flb_sds_destroy(ctx->imds.region); ctx->imds.region = NULL; }
1290-1368: Remove invalid Content-Length header; fix tmp_host/host checks (already ok); avoid double-free
- flb_http_add_header rejects val_len < 1; don’t add Content-Length; client sets it.
- You already guard tmp_host and host; keep as-is.
- Avoid freeing b64_hash both on error and in cleanup.
- flb_http_add_header(client, "Content-Length", 14, NULL, 0); @@ - if (flb_base64_encode - ((unsigned char *) b64_hash, b64_len, &b64_len, hash, - SHA256_DIGEST_LENGTH) != 0) { - flb_free(b64_hash); - goto cleanup; - } + if (flb_base64_encode((unsigned char *) b64_hash, b64_len, &b64_len, hash, SHA256_DIGEST_LENGTH) != 0) { + goto cleanup; + }
1593-1597: Destroy ctx on construct_oci_host failure to avoid leaks- host = construct_oci_host("loganalytics", ctx); - if (!host) { - flb_plg_error(ctx->ins, "failed to construct oci host"); - return NULL; - } + host = construct_oci_host("loganalytics", ctx); + if (!host) { + flb_plg_error(ctx->ins, "failed to construct oci host"); + flb_oci_logan_conf_destroy(ctx); + return NULL; + }plugins/out_oracle_log_analytics/oci_logan.c (4)
556-563: Fix retry condition log checkretry_error() returns FLB_TRUE/FLB_FALSE, not FLB_RETRY.
- if (ret == FLB_RETRY) { + if (ret == FLB_TRUE) { flb_plg_error(ctx->ins, "HTTP %d: will retry", c->resp.status); } else { flb_plg_error(ctx->ins, "HTTP %d: non retryable error", c->resp.status); }
739-747: Use application/json in token-based signing stringThe instance_principal signed string uses application/octet-stream; switch to application/json for JSON body.
- if (payload && payload_size > 0) { - flb_sds_printf(&string_to_sign, - "content-type: application/octet-stream\n"); + if (payload && payload_size > 0) { + flb_sds_printf(&string_to_sign, + "content-type: application/json\n"); flb_sds_printf(&string_to_sign, "content-length: %zu", payload_size);
934-939: Set Content-Type header to application/json when sendingClient header sends application/octet-stream; use application/json for JSON payloads.
- if (payload && payload_size > 0) { - flb_http_add_header(client, "Content-Type", 12, - "application/octet-stream", 24); - } + if (payload && payload_size > 0) { + flb_http_add_header(client, "Content-Type", 12, + "application/json", strlen("application/json")); + }
991-1000: Hash the raw payload digest, not the base64 stringThe hex prefix is computed from the base64 text, which is wrong. Use the raw SHA‑256 digest.
- size_t payload_size = flb_sds_len(payload); - char *content_sha256 = - calculate_content_sha256_b64(payload, payload_size); - if (!content_sha256) { - return; - } - int i; - for (i = 0; i < SHA256_DIGEST_LENGTH; i++) { - sprintf(hash_in_hex + (i * 2), "%02x", content_sha256[i]); - } + size_t payload_size = flb_sds_len(payload); + unsigned char digest[SHA256_DIGEST_LENGTH]; + SHA256((unsigned char *) payload, payload_size, digest); + for (int i = 0; i < SHA256_DIGEST_LENGTH; i++) { + sprintf(hash_in_hex + (i * 2), "%02x", digest[i]); + }
🧹 Nitpick comments (5)
plugins/out_oracle_log_analytics/oci_logan_helper.c (1)
476-482: Ensure helper cleanup runs on plugin exitcleanup_oci_timezone_hash() is never invoked; the hash lives for the process lifetime. Call it from cb_oci_logan_exit() to release memory when the output shuts down.
Follow-up: expose a non-static helper (or a small oci_logan_helper_deinit()) and call it in plugins/out_oracle_log_analytics/oci_logan.c::cb_oci_logan_exit().
plugins/out_oracle_log_analytics/oci_logan.h (2)
175-190: Deduplicate repeated includesMultiple duplicates increase compile time and noise (e.g., flb_upstream.h, flb_hash_table.h, flb_sds.h are included twice).
Remove repeated includes and keep a single, minimal set.
170-170: Avoid header macro referencing a C TU symbolCOUNT_OF_REGION relies on region_mappings[] defined in a .c file. While harmless when unused elsewhere, it’s fragile. Move the macro next to the array in oci_logan_helper.c or expose a constexpr count via a helper.
plugins/out_oracle_log_analytics/oci_logan_conf.c (2)
368-371: Prefer resp.payload over raw resp.dataUse client->resp.payload and client->resp.payload_size to avoid parsing headers and manual body slicing.
- response = flb_sds_create_len(client->resp.data, client->resp.data_len); + response = flb_sds_create_len(client->resp.payload, client->resp.payload_size);
1433-1440: Guard frees in cleanup to be idempotentPrevent double-free if earlier branches jumped to cleanup.
- flb_sds_destroy(date_header); - flb_sds_destroy(url_path); - flb_free(b64_hash); - flb_free(host); - if (client) { - flb_http_client_destroy(client); - } - flb_upstream_conn_release(u_conn); - flb_upstream_destroy(upstream); + if (date_header) flb_sds_destroy(date_header); + if (url_path) flb_sds_destroy(url_path); + if (b64_hash) flb_free(b64_hash); + if (host) flb_free(host); + if (client) flb_http_client_destroy(client); + if (u_conn) flb_upstream_conn_release(u_conn); + if (upstream) flb_upstream_destroy(upstream);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
plugins/out_oracle_log_analytics/CMakeLists.txt(1 hunks)plugins/out_oracle_log_analytics/oci_logan.c(33 hunks)plugins/out_oracle_log_analytics/oci_logan.h(5 hunks)plugins/out_oracle_log_analytics/oci_logan_conf.c(13 hunks)plugins/out_oracle_log_analytics/oci_logan_conf.h(1 hunks)plugins/out_oracle_log_analytics/oci_logan_helper.c(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- plugins/out_oracle_log_analytics/CMakeLists.txt
🧰 Additional context used
🧬 Code graph analysis (5)
plugins/out_oracle_log_analytics/oci_logan_helper.c (1)
src/flb_hash_table.c (4)
flb_hash_table_create(99-137)flb_hash_table_add(401-494)flb_hash_table_destroy(197-215)flb_hash_table_get(496-522)
plugins/out_oracle_log_analytics/oci_logan_conf.h (1)
plugins/out_oracle_log_analytics/oci_logan_conf.c (4)
flb_oci_logan_conf_create(1449-1678)flb_oci_logan_conf_destroy(1712-1754)create_federation_payload(844-883)sign_and_send_federation_request(1290-1447)
plugins/out_oracle_log_analytics/oci_logan_conf.c (8)
plugins/out_oracle_log_analytics/oci_logan.c (2)
EVP_MD_CTX_new(27-30)EVP_MD_CTX_free(32-35)include/fluent-bit/flb_mem.h (2)
flb_free(126-128)flb_calloc(84-96)src/flb_sds.c (6)
flb_sds_destroy(389-399)flb_sds_create_len(58-76)flb_sds_create_size(92-95)flb_sds_snprintf(405-428)flb_sds_create(78-90)flb_sds_printf(336-387)src/flb_http_client.c (4)
flb_http_client(814-859)flb_http_add_header(963-995)flb_http_do(1572-1632)flb_http_client_destroy(1688-1695)plugins/out_oracle_log_analytics/oci_logan_helper.c (3)
determine_realm_from_region(533-545)get_domain_suffix_for_realm(548-561)long_region_name(565-577)src/flb_upstream.c (4)
flb_upstream_create(290-363)flb_upstream_conn_get(711-844)flb_upstream_conn_release(862-947)flb_upstream_destroy(656-698)src/flb_output.c (1)
flb_output_net_default(1059-1069)src/flb_utils.c (1)
flb_utils_url_split(1441-1534)
plugins/out_oracle_log_analytics/oci_logan.h (1)
plugins/out_oracle_log_analytics/oci_logan_helper.c (4)
is_valid_timezone(520-530)get_domain_suffix_for_realm(548-561)determine_realm_from_region(533-545)long_region_name(565-577)
plugins/out_oracle_log_analytics/oci_logan.c (7)
plugins/out_oracle_log_analytics/oci_logan_conf.c (4)
EVP_MD_CTX_new(43-46)EVP_MD_CTX_free(48-51)create_federation_payload(844-883)sign_and_send_federation_request(1290-1447)src/flb_sds.c (6)
flb_sds_create_size(92-95)flb_sds_cat_safe(204-214)flb_sds_destroy(389-399)flb_sds_create(78-90)flb_sds_printf(336-387)flb_sds_create_len(58-76)src/flb_http_client.c (5)
flb_http_add_header(963-995)flb_http_client(814-859)flb_http_client_destroy(1688-1695)flb_http_allow_duplicated_headers(99-107)flb_http_buffer_size(872-883)src/flb_upstream.c (1)
flb_upstream_conn_get(711-844)src/flb_log_event_decoder.c (3)
flb_log_event_decoder_init(99-116)flb_log_event_decoder_next(310-406)flb_log_event_decoder_destroy(147-179)src/flb_pack.c (1)
flb_msgpack_raw_to_json_sds(1026-1085)src/flb_mp.c (1)
flb_mp_count(43-46)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (30)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_ARROW=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
- GitHub Check: pr-compile-without-cxx (3.31.6)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
- GitHub Check: PR - fuzzing test
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-centos-7
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-22.04, clang-12)
yes its been tested, i pushed tests for both authentications |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/out_oracle_log_analytics/oci_logan_conf.c (1)
108-111: Off‑by‑one: SDS length should exclude the NUL terminatorStore len as file size, not size+1. Current code reports one extra byte.
/* In mbedtls, for PEM, the buffer must contains a null-terminated string */ kbuffer[finfo.size] = '\0'; - flb_sds_len_set(kbuffer, finfo.size + 1); + flb_sds_len_set(kbuffer, finfo.size);
♻️ Duplicate comments (6)
plugins/out_oracle_log_analytics/oci_logan_conf.c (6)
1386-1389: Remove invalid Content‑Length header (client sets it)- flb_http_add_header(client, "Content-Length", 14, NULL, 0);Based on learnings
161-165: Free ‘content’ on empty file to avoid leak- if (content == NULL || flb_sds_len(content) == 0) { - return -1; - } + if (content == NULL || flb_sds_len(content) == 0) { + if (content) { flb_sds_destroy(content); } + return -1; + }Based on learnings
663-676: Free key_resp after extracting PEM; avoid leaksize_t pem_len = (pem_end - pem_start) + strlen("-----END RSA PRIVATE KEY-----") + 1; ctx->imds.leaf_key = flb_sds_create_len(pem_start, pem_len); + /* key_resp contains the HTTP body; safe to destroy after copying the PEM */ + flb_sds_destroy(key_resp);Based on learnings
686-709: Error path: free IMDS allocations instead of NULLing- ctx->imds.fingerprint = NULL; - ctx->imds.intermediate_cert = NULL; - ctx->imds.leaf_cert = NULL; - ctx->imds.leaf_key = NULL; - ctx->imds.region = NULL; + if (ctx->imds.fingerprint) { + flb_sds_destroy(ctx->imds.fingerprint); + ctx->imds.fingerprint = NULL; + } + if (ctx->imds.intermediate_cert) { + flb_sds_destroy(ctx->imds.intermediate_cert); + ctx->imds.intermediate_cert = NULL; + } + if (ctx->imds.leaf_cert) { + flb_free(ctx->imds.leaf_cert); + ctx->imds.leaf_cert = NULL; + } + if (ctx->imds.leaf_key) { + flb_sds_destroy(ctx->imds.leaf_key); + ctx->imds.leaf_key = NULL; + } + if (ctx->imds.region) { + flb_sds_destroy(ctx->imds.region); + ctx->imds.region = NULL; + }Based on learnings
1401-1407: Fix double‑free of b64_hash and guard cleanupif (flb_base64_encode ((unsigned char *) b64_hash, b64_len, &b64_len, hash, SHA256_DIGEST_LENGTH) != 0) { - flb_free(b64_hash); + flb_free(b64_hash); + b64_hash = NULL; goto cleanup; } @@ - flb_free(b64_hash); + if (b64_hash) { flb_free(b64_hash); }Based on learnings
Also applies to: 1458-1461
1616-1619: Destroy ctx on construct_oci_host failure to avoid leakhost = construct_oci_host("loganalytics", ctx); if (!host) { flb_plg_error(ctx->ins, "failed to construct oci host"); - return NULL; + flb_oci_logan_conf_destroy(ctx); + return NULL; }Based on learnings
🧹 Nitpick comments (5)
plugins/out_oracle_log_analytics/oci_logan_conf.c (5)
380-383: Prefer response payload over raw bufferUsing client->resp.payload avoids manual header parsing and is safer.
- response = flb_sds_create_len(client->resp.data, client->resp.data_len); + response = flb_sds_create_len(client->resp.payload, client->resp.payload_size);
938-941: Use SDS length for content-length in signature- flb_sds_printf(&string_to_sign, "content-length: %zu\n", - (payload) ? strlen(payload) : 0); + flb_sds_printf(&string_to_sign, "content-length: %zu\n", + (payload) ? flb_sds_len(payload) : 0);
1105-1110: Handle JSMN_ERROR_NOMEM and generic parse errors- if (ret == JSMN_ERROR_INVAL || ret == JSMN_ERROR_PART) { + if (ret < 0) { flb_free(tokens); return -1; }
1370-1373: Use SDS length for HTTP body length- client = flb_http_client(u_conn, FLB_HTTP_POST, url_path, - payload, strlen(payload), host, port, NULL, 0); + client = flb_http_client(u_conn, FLB_HTTP_POST, url_path, + payload, flb_sds_len(payload), host, port, NULL, 0);
1461-1466: Guard connection cleanup calls- if (client) { - flb_http_client_destroy(client); - } - flb_upstream_conn_release(u_conn); - flb_upstream_destroy(upstream); + if (client) { + flb_http_client_destroy(client); + } + if (u_conn) { + flb_upstream_conn_release(u_conn); + } + if (upstream) { + flb_upstream_destroy(upstream); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
plugins/out_oracle_log_analytics/oci_logan_conf.c(6 hunks)tests/runtime/CMakeLists.txt(1 hunks)tests/runtime/out_oracle_log_analytics.c(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
tests/runtime/out_oracle_log_analytics.c (2)
plugins/out_s3/s3.c (1)
setenv(57-60)src/flb_lib.c (10)
flb_create(138-220)flb_service_set(647-673)flb_input(261-271)flb_input_set(300-330)flb_output(274-284)flb_output_set(515-546)flb_start(914-925)flb_lib_push(774-801)flb_stop(942-985)flb_destroy(223-258)
plugins/out_oracle_log_analytics/oci_logan_conf.c (7)
include/fluent-bit/flb_mem.h (2)
flb_free(126-128)flb_calloc(84-96)src/flb_sds.c (4)
flb_sds_destroy(389-399)flb_sds_create_len(58-76)flb_sds_create(78-90)flb_sds_printf(336-387)src/flb_http_client.c (4)
flb_http_client(814-859)flb_http_add_header(963-995)flb_http_do(1572-1632)flb_http_client_destroy(1688-1695)plugins/out_oracle_log_analytics/oci_logan_helper.c (3)
determine_realm_from_region(533-545)get_domain_suffix_for_realm(548-561)long_region_name(565-577)src/flb_upstream.c (4)
flb_upstream_create(290-363)flb_upstream_conn_get(711-844)flb_upstream_conn_release(862-947)flb_upstream_destroy(656-698)include/fluent-bit/flb_output.h (1)
flb_output_config_map_set(1298-1321)src/flb_output.c (2)
flb_output_net_default(1059-1069)flb_output_upstream_set(1556-1647)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-without-cxx (3.31.6)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-centos-7
- GitHub Check: PR - fuzzing test
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (11)
plugins/out_oracle_log_analytics/oci_logan_conf.c (11)
162-163: Free empty content before returning.When
flb_file_read()returns an empty SDS (length 0), the buffer is leaked. This was flagged in a previous review but appears unaddressed.Apply this diff:
- if (content == NULL || flb_sds_len(content) == 0) { + if (content == NULL || flb_sds_len(content) == 0) { + if (content) { + flb_sds_destroy(content); + } return -1; }
1621-1625: Destroy ctx on construct_oci_host failure.When
construct_oci_hostfails at line 1621, the function returns NULL without freeingctx, leaking all previously allocated resources. This was flagged in a previous review but remains unaddressed.Apply this diff:
host = construct_oci_host("loganalytics", ctx); if (!host) { flb_plg_error(ctx->ins, "failed to construct oci host"); + flb_oci_logan_conf_destroy(ctx); return NULL; }
1394-1394: Remove invalid Content-Length header.
flb_http_add_headerrejects headers withval_len < 1, so this call silently fails. The HTTP client automatically manages Content-Length, making this unnecessary. This was flagged in a previous review but remains unaddressed.Apply this diff:
- flb_http_add_header(client, "Content-Length", 14, NULL, 0);
661-669: Free key_resp after extracting the PEM block.After creating
ctx->imds.leaf_keyat line 669, thekey_respSDS is leaked. This was flagged in a previous review but remains unaddressed.Add this line after line 669:
ctx->imds.leaf_key = flb_sds_create_len(pem_start, pem_len); + flb_sds_destroy(key_resp); // extract tenancy ocid from leaf certificate
1326-1330: Check tmp_host before use to avoid NPE.
construct_oci_host()can return NULL (e.g., when service or region is NULL). Line 1331 callsflb_sds_len(tmp_host)without checking, which will cause a null pointer dereference. This was flagged in a previous review but remains unaddressed.Apply this diff:
flb_sds_t url_path = flb_sds_create("/v1/x509"); flb_sds_t auth_header = NULL; flb_sds_t date_header = NULL; flb_sds_t tmp_host = construct_oci_host("auth", ctx); + if (!tmp_host) { + flb_sds_destroy(url_path); + return NULL; + } char *host = flb_calloc(flb_sds_len(tmp_host) + 1, 1);
692-716: Free partially allocated IMDS fields on error.The error cleanup path (lines 707-711) only NULLs the IMDS pointers without freeing them. If some fields were successfully allocated before the error, this causes a memory leak. This was flagged in a previous review but remains unaddressed.
Apply this diff:
error: if (!is_test_mode()) { if (region_resp) { flb_sds_destroy(region_resp); } if (cert_resp) { flb_sds_destroy(cert_resp);; } if (key_resp) { flb_sds_destroy(key_resp); } if (int_cert_resp) { flb_sds_destroy(int_cert_resp); } - ctx->imds.fingerprint = NULL; - ctx->imds.intermediate_cert = NULL; - ctx->imds.leaf_cert = NULL; - ctx->imds.leaf_key = NULL; - ctx->imds.region = NULL; + if (ctx->imds.fingerprint) { + flb_sds_destroy(ctx->imds.fingerprint); + ctx->imds.fingerprint = NULL; + } + if (ctx->imds.intermediate_cert) { + flb_sds_destroy(ctx->imds.intermediate_cert); + ctx->imds.intermediate_cert = NULL; + } + if (ctx->imds.leaf_cert) { + flb_free(ctx->imds.leaf_cert); + ctx->imds.leaf_cert = NULL; + } + if (ctx->imds.leaf_key) { + flb_sds_destroy(ctx->imds.leaf_key); + ctx->imds.leaf_key = NULL; + } + if (ctx->imds.region) { + flb_sds_destroy(ctx->imds.region); + ctx->imds.region = NULL; + } flb_upstream_conn_release(u_conn); flb_upstream_destroy(ctx->u); ctx->u = NULL; } return 0; }
1740-1782: Complete destructor: free all allocated IMDS and session resources.The destructor is missing cleanup for several instance_principal-related fields, causing memory leaks. This was flagged in a previous review but remains unaddressed.
Apply this diff:
int flb_oci_logan_conf_destroy(struct flb_oci_logan *ctx) { if (ctx == NULL) { return 0; } + /* IMDS / federation artifacts */ + if (ctx->imds.fingerprint) { + flb_sds_destroy(ctx->imds.fingerprint); + } + if (ctx->imds.intermediate_cert) { + flb_sds_destroy(ctx->imds.intermediate_cert); + } + if (ctx->imds.leaf_cert) { + flb_free(ctx->imds.leaf_cert); + } + if (ctx->imds.leaf_key) { + flb_sds_destroy(ctx->imds.leaf_key); + } + if (ctx->imds.region) { + flb_sds_destroy(ctx->imds.region); + } + if (ctx->imds.tenancy_ocid) { + flb_sds_destroy(ctx->imds.tenancy_ocid); + } + if (ctx->imds.session_pubkey) { + flb_free(ctx->imds.session_pubkey); + } + if (ctx->imds.session_privkey) { + flb_free(ctx->imds.session_privkey); + } + if (ctx->session_key_pair) { + EVP_PKEY_free(ctx->session_key_pair); + } + if (ctx->security_token.token) { + flb_sds_destroy(ctx->security_token.token); + } + if (ctx->proxy_host) { + flb_free(ctx->proxy_host); + } + if (ctx->private_key) { flb_sds_destroy(ctx->private_key); }
722-738: Check allocations and fix leak on OpenSSL <3.0 path.The OpenSSL 1.x code path has two issues flagged in a previous review that remain unaddressed:
- Missing NULL checks:
EVP_PKEY_new(),BN_new(), andRSA_new()can return NULL but are not checked.- Leak on failure: If
RSA_generate_key_ex()fails,pkeyis not freed (lines 731-733).Apply this diff:
#if OPENSSL_VERSION_NUMBER < 0x30000000L EVP_PKEY *pkey = EVP_PKEY_new(); BIGNUM *bn = BN_new(); RSA *rsa = RSA_new(); int rc; + if (!pkey || !bn || !rsa) { + if (rsa) { RSA_free(rsa); } + if (bn) { BN_free(bn); } + if (pkey){ EVP_PKEY_free(pkey); } + return NULL; + } + BN_set_word(bn, RSA_F4); rc = RSA_generate_key_ex(rsa, 2048, bn, NULL); if (rc != 1) { RSA_free(rsa); BN_free(bn); + EVP_PKEY_free(pkey); return NULL; }
415-451: Fix region buffer leak and support body-only responses.Two issues from a previous review remain unaddressed:
- Memory leak: The temporary
malloc'd bufferregionis not freed on the success path (line 450).- Missing body-only support: When IMDS returns a body without headers (no
\r\n\r\n), the function returnsNULLinstead of treating the entire response as the body.Apply this diff:
flb_sds_t extract_region(const char *response) { const char *body_start = strstr(response, "\r\n\r\n"); - if (!body_start) { - return NULL; - } + if (body_start) { + body_start += 4; + } + else { + /* No headers: treat full input as body */ + body_start = response; + } - body_start += 4; - while (*body_start == '\n' || *body_start == '\r' || *body_start == ' ') { body_start++; } @@ const char *region_value = long_name ? long_name : region; flb_sds_t lregion = flb_sds_create(region_value); if (!lregion) { free(region); return NULL; } + free(region); return lregion; }
1406-1411: Fix double-free of b64_hash.If base64 encoding fails,
b64_hashis freed at line 1409, then freed again in the cleanup section at line 1465, causing a double-free.Apply this diff:
if (flb_base64_encode ((unsigned char *) b64_hash, b64_len, &b64_len, hash, SHA256_DIGEST_LENGTH) != 0) { - flb_free(b64_hash); goto cleanup; }
1322-1339: Short-circuit test mode before allocating resources.The test mode check at line 1336 happens after constructing
tmp_hostand allocatinghost, which can fail when region is mocked. Move the test mode check earlier to avoid unnecessary allocations and potential failures. This was flagged in a previous review.Apply this diff:
flb_sds_t resp = NULL; int port = 443; + if (is_test_mode()) { + return mock_federation_response(ctx); + } flb_sds_t url_path = flb_sds_create("/v1/x509"); flb_sds_t auth_header = NULL; flb_sds_t date_header = NULL; flb_sds_t tmp_host = construct_oci_host("auth", ctx); @@ char *host = flb_calloc(flb_sds_len(tmp_host) + 1, 1); time_t now; struct tm *tm_info; char date_buf[128]; - if (is_test_mode()) { - flb_sds_destroy(url_path); - return mock_federation_response(ctx); - }
🧹 Nitpick comments (1)
plugins/out_oracle_log_analytics/oci_logan_conf.c (1)
1459-1471: Guard b64_hash before free to avoid issues.Although
free(NULL)is safe, for consistency and clarity in cleanup code, guardb64_hashbefore freeing.Apply this diff:
cleanup: @@ flb_sds_destroy(date_header); flb_sds_destroy(url_path); - flb_free(b64_hash); + if (b64_hash) { + flb_free(b64_hash); + } flb_free(host);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
plugins/out_oracle_log_analytics/oci_logan_conf.c(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
plugins/out_oracle_log_analytics/oci_logan_conf.c (6)
include/fluent-bit/flb_mem.h (2)
flb_free(126-128)flb_calloc(84-96)src/flb_sds.c (6)
flb_sds_destroy(389-399)flb_sds_create_len(58-76)flb_sds_create_size(92-95)flb_sds_snprintf(405-428)flb_sds_create(78-90)flb_sds_printf(336-387)src/flb_http_client.c (4)
flb_http_client(814-859)flb_http_add_header(963-995)flb_http_do(1572-1632)flb_http_client_destroy(1688-1695)plugins/out_oracle_log_analytics/oci_logan_helper.c (3)
determine_realm_from_region(533-545)get_domain_suffix_for_realm(548-561)long_region_name(565-577)src/flb_upstream.c (4)
flb_upstream_create(290-363)flb_upstream_conn_get(711-844)flb_upstream_conn_release(862-947)flb_upstream_destroy(656-698)src/flb_utils.c (1)
flb_utils_url_split(1441-1534)
🪛 Gitleaks (8.28.0)
plugins/out_oracle_log_analytics/oci_logan_conf.c
[high] 1818-1824: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.
(private-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (30)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
- GitHub Check: pr-compile-without-cxx (3.31.6)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-centos-7
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_ARROW=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: PR - fuzzing test
🔇 Additional comments (13)
plugins/out_oracle_log_analytics/oci_logan_conf.c (13)
117-150: LGTM!The
trim_whitespaceimplementation correctly handles edge cases (NULL input, empty strings, whitespace-only strings) and uses consistent allocation withflb_malloc.
348-383: LGTM!The IMDS request implementation correctly handles test mode, creates HTTP client, checks response status, and cleans up resources.
385-413: LGTM!The
construct_oci_hostfunction correctly validates inputs, uses helper functions for realm/domain lookup, and handles allocation failures.
453-478: LGTM!The
extract_pem_contentfunction correctly locates PEM markers, allocates memory, and copies the PEM block.
480-535: LGTM!The fingerprint calculation correctly uses OpenSSL APIs, checks all allocations, handles errors, and cleans up resources properly.
537-593: LGTM!The tenancy OCID extraction now correctly handles ASN.1 strings using length-aware operations (
memcmp,memchr,flb_sds_create_len) rather than assuming NUL-termination. Previous review concerns have been addressed.Based on learnings
770-829: LGTM!Both
extract_public_key_pemandextract_private_key_pemcorrectly use OpenSSL BIO APIs, check allocations, and clean up resources.
831-864: LGTM!The
sanitize_certificatefunction correctly extracts the certificate body, removes whitespace, and returns a sanitized SDS string with proper NULL handling.
866-906: LGTM!The federation payload creation now correctly guards against NULL returns from
sanitize_certificate(lines 884-886) and eliminates the redundant assignment. Previous review concerns have been addressed.Based on learnings
908-1061: LGTM!The request signing function correctly uses OpenSSL EVP digest signing APIs, checks all allocations, and ensures thorough cleanup of allocated resources in the cleanup section.
1063-1082: LGTM!The token string cleaning function correctly filters non-printable characters and updates the SDS length in place.
1084-1158: LGTM!The federation response parsing correctly uses jsmn to extract the token, cleans it, replaces any existing token, and frees allocated resources.
1160-1310: LGTM!The JWT decoding function correctly validates JWT format, performs base64url decoding with proper padding, parses the JSON payload, extracts the expiration time, and cleans up all temporary allocations.
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/runtime/out_oracle_log_analytics.c(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/runtime/out_oracle_log_analytics.c (2)
plugins/out_s3/s3.c (1)
setenv(57-60)src/flb_lib.c (9)
flb_create(138-220)flb_service_set(647-673)flb_input(261-271)flb_destroy(223-258)flb_input_set(300-330)flb_output(274-284)flb_output_set(515-546)flb_start(914-925)flb_stop(942-985)
🪛 Gitleaks (8.28.0)
tests/runtime/out_oracle_log_analytics.c
[high] 134-160: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.
(private-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (30)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_ARROW=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
- GitHub Check: PR - fuzzing test
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-without-cxx (3.31.6)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-centos-7
🔇 Additional comments (2)
tests/runtime/out_oracle_log_analytics.c (2)
134-160: Static analysis false positive: Test key is intentional.The hardcoded RSA private key is a test fixture, not a real credential. This is standard practice for authentication tests.
212-217: LGTM: Test registration follows framework conventions.The
TEST_LISTproperly registers all three test functions for the runtime test framework.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (6)
plugins/out_oracle_log_analytics/oci_logan_conf.c (6)
1346-1364: Critical: NPE risk - check tmp_host before use.
construct_oci_host()can return NULL, butflb_sds_len(tmp_host)is called before checking.Apply this fix:
flb_sds_t tmp_host = construct_oci_host("auth", ctx); + if (!tmp_host) { + flb_sds_destroy(url_path); + return NULL; + } char *host = flb_calloc(flb_sds_len(tmp_host) + 1, 1);Based on learnings
1410-1410: Remove invalid Content-Length header call.
flb_http_add_headerrejects headers withval_len < 1. This call silently fails and is unnecessary since the HTTP client sets Content-Length automatically.- flb_http_add_header(client, "Content-Length", 14, NULL, 0);Based on learnings
1637-1641: Major: Memory leak on host construction failure.When
construct_oci_host()fails,ctxis not freed before returning NULL.Apply this fix:
host = construct_oci_host("loganalytics", ctx); if (!host) { flb_plg_error(ctx->ins, "failed to construct oci host"); + flb_oci_logan_conf_destroy(ctx); return NULL; }Based on learnings
1475-1487: Critical: Double-free and NULL dereference risks in cleanup.
b64_hashis conditionally freed beforegoto cleanupbut freed again unconditionally in cleanup. Also,clientmay be NULL at some goto sites.Apply this fix:
flb_plg_error(ctx->ins, "failed to parse federation response"); flb_sds_destroy(resp); resp = NULL; - flb_free(b64_hash); goto cleanup; } decode_jwt_and_set_expires(ctx); } cleanup: if (auth_header) { flb_sds_destroy(auth_header); } flb_sds_destroy(date_header); flb_sds_destroy(url_path); - flb_free(b64_hash); + if (b64_hash) { + flb_free(b64_hash); + } flb_free(host); - flb_http_client_destroy(client); + if (client) { + flb_http_client_destroy(client); + } flb_upstream_conn_release(u_conn); flb_upstream_destroy(upstream);Based on learnings
708-731: Critical: Incomplete error cleanup in get_keys_and_certs.The error path only NULLs the IMDS pointers without freeing allocated memory. Also,
ctx->imds.leaf_certshould useflb_free(notflb_sds_destroy) since it's allocated withflb_callocviaextract_pem_content.Apply this fix:
error: if (!is_test_mode()) { if (region_resp) { flb_sds_destroy(region_resp); } if (cert_resp) { - flb_sds_destroy(cert_resp);; + flb_sds_destroy(cert_resp); } if (key_resp) { flb_sds_destroy(key_resp); } if (int_cert_resp) { flb_sds_destroy(int_cert_resp); } - ctx->imds.fingerprint = NULL; - ctx->imds.intermediate_cert = NULL; - ctx->imds.leaf_cert = NULL; - ctx->imds.leaf_key = NULL; - ctx->imds.region = NULL; + if (ctx->imds.fingerprint) { + flb_sds_destroy(ctx->imds.fingerprint); + ctx->imds.fingerprint = NULL; + } + if (ctx->imds.intermediate_cert) { + flb_sds_destroy(ctx->imds.intermediate_cert); + ctx->imds.intermediate_cert = NULL; + } + if (ctx->imds.leaf_cert) { + flb_free(ctx->imds.leaf_cert); + ctx->imds.leaf_cert = NULL; + } + if (ctx->imds.leaf_key) { + flb_sds_destroy(ctx->imds.leaf_key); + ctx->imds.leaf_key = NULL; + } + if (ctx->imds.region) { + flb_sds_destroy(ctx->imds.region); + ctx->imds.region = NULL; + } flb_upstream_conn_release(u_conn); flb_upstream_destroy(ctx->u); ctx->u = NULL; } return 0; }Based on learnings
738-754: Check allocation results in OpenSSL <3.0 path.The legacy OpenSSL code path doesn't verify that
EVP_PKEY_new(),BN_new(), orRSA_new()succeeded before use.Apply this fix:
#if OPENSSL_VERSION_NUMBER < 0x30000000L EVP_PKEY *pkey = EVP_PKEY_new(); BIGNUM *bn = BN_new(); RSA *rsa = RSA_new(); int rc; + if (!pkey || !bn || !rsa) { + if (rsa) { RSA_free(rsa); } + if (bn) { BN_free(bn); } + if (pkey){ EVP_PKEY_free(pkey); } + return NULL; + } + BN_set_word(bn, RSA_F4); rc = RSA_generate_key_ex(rsa, 2048, bn, NULL); if (rc != 1) { RSA_free(rsa); BN_free(bn); + EVP_PKEY_free(pkey); return NULL; }Based on learnings
🧹 Nitpick comments (1)
plugins/out_oracle_log_analytics/oci_logan_conf.c (1)
470-477: Limit PEM size before allocation
Add a sanity check onpem_lengthto avoid unbounded allocations whenresponsecontains malformed or maliciously large data. For example, beforeflb_calloc, do:if (pem_length > MAX_PEM_SIZE) { flb_plg_error(ctx->ins, "PEM block too large (%zu bytes)", pem_length); return NULL; }where
MAX_PEM_SIZEis a reasonable upper bound (e.g. 64KB).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
plugins/out_oracle_log_analytics/oci_logan_conf.c(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
plugins/out_oracle_log_analytics/oci_logan_conf.c (7)
include/fluent-bit/flb_mem.h (2)
flb_free(126-128)flb_calloc(84-96)src/flb_sds.c (5)
flb_sds_destroy(389-399)flb_sds_create_len(58-76)flb_sds_create_size(92-95)flb_sds_create(78-90)flb_sds_printf(336-387)src/flb_http_client.c (4)
flb_http_client(814-859)flb_http_add_header(963-995)flb_http_do(1572-1632)flb_http_client_destroy(1688-1695)plugins/out_oracle_log_analytics/oci_logan_helper.c (3)
determine_realm_from_region(533-545)get_domain_suffix_for_realm(548-561)long_region_name(565-577)src/flb_upstream.c (4)
flb_upstream_create(290-363)flb_upstream_conn_get(711-844)flb_upstream_conn_release(862-947)flb_upstream_destroy(656-698)src/flb_output.c (2)
flb_output_net_default(1059-1069)flb_output_upstream_set(1556-1647)src/flb_utils.c (1)
flb_utils_url_split(1441-1534)
🪛 Gitleaks (8.28.0)
plugins/out_oracle_log_analytics/oci_logan_conf.c
[high] 614-683: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.
(private-key)
[high] 1862-1868: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.
(private-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (30)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
- GitHub Check: run-ubuntu-unit-tests (-DFLB_ARROW=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
- GitHub Check: PR - fuzzing test
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-centos-7
- GitHub Check: pr-compile-without-cxx (3.31.6)
🔇 Additional comments (12)
plugins/out_oracle_log_analytics/oci_logan_conf.c (12)
39-53: LGTM: OpenSSL compatibility layer.The compatibility macros for OpenSSL <1.1.0 are correctly defined and will enable compilation on older systems.
117-150: LGTM: Whitespace trimming implementation.The
trim_whitespacefunction correctly handles edge cases (NULL, empty strings) and properly allocates/copies the trimmed result.
196-232: LGTM: Credential parsing fixes applied.The key/value extraction now correctly:
- Saves original key before trimming
- Frees trimmed values after creating SDS copies
- Uses consistent memory management (mk_mem_free for mk_string_copy_substr, flb_free for flb_malloc)
Based on learnings
416-451: Verify: Memory leak in extract_region.The temporary
regionbuffer allocated withmallocis not freed before returninglregion.Apply this fix to prevent the leak:
flb_sds_t lregion = flb_sds_create(region_value); if (!lregion) { free(region); return NULL; } + free(region); return lregion; }Based on learnings
542-598: LGTM: Safe ASN1 parsing implemented.The tenancy OCID extraction now correctly:
- Uses
ASN1_STRING_length()for bounds checking- Uses
memcmp()for length-aware comparison- Uses
memchr()for safe colon search- Creates SDS with explicit length
Based on learnings
665-684: Verify: Missing free of key_resp SDS.After extracting the PEM block into
ctx->imds.leaf_key, the originalkey_respSDS is not destroyed.Apply this fix:
size_t pem_len = (pem_end - pem_start) + strlen("-----END RSA PRIVATE KEY-----") + 1; ctx->imds.leaf_key = flb_sds_create_len(pem_start, pem_len); + flb_sds_destroy(key_resp);Based on learnings
883-922: LGTM: Proper NULL checks in create_federation_payload.The function now correctly verifies
leaf_certandsession_pubkeyare non-NULL before using them inflb_sds_printf, and properly cleans up all temporary SDS strings.Based on learnings
1762-1790: LGTM: Comprehensive destructor with proper cleanup.The destructor now properly frees all IMDS-related resources including:
- Region, keys, certificates (using appropriate free functions)
- Session keys (using
free()for malloc'd buffers)- EVP_PKEY (using
EVP_PKEY_free())- Security token
Based on learnings
1829-1939: Note: Test mock functions for development.The mock functions provide test data for IMDS and federation responses. The private keys flagged by Gitleaks are intentionally fake test data.
Note: Gitleaks warnings at lines 614-683 and 1862-1868 are false positives—these are mock test keys, not real credentials.
1694-1698: Confirm proxy_host allocation
flb_utils_url_split()returns a heap-allocated host string; assigningp_hosttoctx->proxy_hostwithout copying is safe.
358-364: Remove incorrect conditional‐compilation concern. Bothis_test_mode()(line 1830) andmock_imds_request()(line 1833) are defined unconditionally, so mocks are always compiled.Likely an incorrect or invalid review comment.
1676-1680: No action needed for conditional host destruction logic. construct_oci_host() always returns a distinct SDS buffer and flb_output_net_default() copies its contents into ins->host.name, so the subsequent flb_sds_destroy(host) correctly frees the temporary allocation.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (5)
plugins/out_oracle_log_analytics/oci_logan_conf.c (5)
1410-1410: Remove invalid Content-Length header addition.
flb_http_add_headerrejects headers withval_len < 1. This call withNULLvalue and0length fails silently and is unnecessary since the HTTP client automatically sets Content-Length when a body is provided.Apply this diff:
- flb_http_add_header(client, "Content-Length", 14, NULL, 0);
436-451: Free temporary region buffer to prevent leak.The
regionbuffer allocated at line 436 is never freed on the success path. After creatinglregion(which copies the data), the temporary buffer must be freed.Apply this diff:
flb_sds_t lregion = flb_sds_create(region_value); if (!lregion) { free(region); return NULL; } + free(region); return lregion; }
162-164: Free empty SDS before returning.When
flb_file_read()returns a non-NULL SDS with length 0, the allocated buffer leaks because it's not freed before returning.Apply this diff:
- if (content == NULL || flb_sds_len(content) == 0) { + if (content == NULL || flb_sds_len(content) == 0) { + if (content) { + flb_sds_destroy(content); + } return -1; }
676-685: Destroy key_resp after extracting PEM block.After line 684 creates
ctx->imds.leaf_keyby copying the PEM block fromkey_resp, the originalkey_respSDS is never freed, causing a memory leak.Apply this diff:
size_t pem_len = (pem_end - pem_start) + strlen("-----END RSA PRIVATE KEY-----") + 1; ctx->imds.leaf_key = flb_sds_create_len(pem_start, pem_len); + flb_sds_destroy(key_resp);
708-732: Free allocated resources in error path instead of just NULLing.Lines 723-727 only set IMDS pointers to NULL without freeing previously allocated strings, causing memory leaks on partial failure.
Apply this diff:
- ctx->imds.fingerprint = NULL; - ctx->imds.intermediate_cert = NULL; - ctx->imds.leaf_cert = NULL; - ctx->imds.leaf_key = NULL; - ctx->imds.region = NULL; + if (ctx->imds.fingerprint) { + flb_sds_destroy(ctx->imds.fingerprint); + ctx->imds.fingerprint = NULL; + } + if (ctx->imds.intermediate_cert) { + flb_sds_destroy(ctx->imds.intermediate_cert); + ctx->imds.intermediate_cert = NULL; + } + if (ctx->imds.leaf_cert) { + flb_free(ctx->imds.leaf_cert); + ctx->imds.leaf_cert = NULL; + } + if (ctx->imds.leaf_key) { + flb_sds_destroy(ctx->imds.leaf_key); + ctx->imds.leaf_key = NULL; + } + if (ctx->imds.region) { + flb_sds_destroy(ctx->imds.region); + ctx->imds.region = NULL; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
plugins/out_oracle_log_analytics/oci_logan_conf.c(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
plugins/out_oracle_log_analytics/oci_logan_conf.c (7)
include/fluent-bit/flb_mem.h (1)
flb_free(126-128)src/flb_sds.c (6)
flb_sds_destroy(389-399)flb_sds_create_len(58-76)flb_sds_create_size(92-95)flb_sds_snprintf(405-428)flb_sds_create(78-90)flb_sds_printf(336-387)src/flb_http_client.c (4)
flb_http_client(814-859)flb_http_add_header(963-995)flb_http_do(1572-1632)flb_http_client_destroy(1688-1695)plugins/out_oracle_log_analytics/oci_logan_helper.c (3)
determine_realm_from_region(533-545)get_domain_suffix_for_realm(548-561)long_region_name(565-577)src/flb_upstream.c (4)
flb_upstream_create(290-363)flb_upstream_conn_get(711-844)flb_upstream_conn_release(862-947)flb_upstream_destroy(656-698)include/fluent-bit/flb_output.h (1)
flb_output_config_map_set(1298-1321)src/flb_output.c (2)
flb_output_net_default(1059-1069)flb_output_upstream_set(1556-1647)
🪛 Gitleaks (8.28.0)
plugins/out_oracle_log_analytics/oci_logan_conf.c
[high] 614-683: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.
(private-key)
[high] 1864-1870: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.
(private-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (30)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_ARROW=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-centos-7
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-without-cxx (3.31.6)
- GitHub Check: PR - fuzzing test
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/out_oracle_log_analytics/oci_logan.h (1)
115-119: Use application/json as Content-TypeUploads are JSON; make the default consistent across signing/requests.
-#define FLB_OCI_HEADER_CONTENT_TYPE_VAL "application/octet-stream" +#define FLB_OCI_HEADER_CONTENT_TYPE_VAL "application/json"
♻️ Duplicate comments (17)
tests/runtime/out_oracle_log_analytics.c (2)
160-172: Verify fwrite() results to ensure files are valid- if (config_file) { - fwrite(config_content, 1, strlen(config_content), config_file); - fclose(config_file); - } + if (config_file) { + size_t written = fwrite(config_content, 1, strlen(config_content), config_file); + TEST_CHECK(written == strlen(config_content)); + fclose(config_file); + } @@ - if (key_file) { - fwrite(key_content, 1, strlen(key_content), key_file); - fclose(key_file); - } + if (key_file) { + size_t written = fwrite(key_content, 1, strlen(key_content), key_file); + TEST_CHECK(written == strlen(key_content)); + fclose(key_file); + }Also applies to: 167-172
72-75: Unset FLB_OCI_PLUGIN_UNDER_TEST to avoid state leakFirst test doesn't unset the flag; leaks into later tests.
flb_destroy(ctx); - unsetenv("TEST_IMDS_SUCCESS"); + unsetenv("FLB_OCI_PLUGIN_UNDER_TEST"); + unsetenv("TEST_IMDS_SUCCESS"); }plugins/out_oracle_log_analytics/oci_logan_conf.c (9)
162-166: Free SDS on empty content to avoid leak- if (content == NULL || flb_sds_len(content) == 0) { - return -1; - } + if (content == NULL || flb_sds_len(content) == 0) { + if (content) { flb_sds_destroy(content); } + return -1; + }
738-746: Error path: release partial IMDS allocations instead of NULLingAvoid leaks on failures after partial success.
- ctx->imds.fingerprint = NULL; - ctx->imds.intermediate_cert = NULL; - ctx->imds.leaf_cert = NULL; - ctx->imds.leaf_key = NULL; - ctx->imds.region = NULL; + if (ctx->imds.fingerprint) { flb_sds_destroy(ctx->imds.fingerprint); ctx->imds.fingerprint = NULL; } + if (ctx->imds.intermediate_cert) { flb_sds_destroy(ctx->imds.intermediate_cert); ctx->imds.intermediate_cert = NULL; } + if (ctx->imds.leaf_cert) { flb_free(ctx->imds.leaf_cert); ctx->imds.leaf_cert = NULL; } + if (ctx->imds.leaf_key) { flb_sds_destroy(ctx->imds.leaf_key); ctx->imds.leaf_key = NULL; } + if (ctx->imds.region) { flb_sds_destroy(ctx->imds.region); ctx->imds.region = NULL; }
754-769: Check OpenSSL allocations (<3.0) and free on failure#if OPENSSL_VERSION_NUMBER < 0x30000000L EVP_PKEY *pkey = EVP_PKEY_new(); BIGNUM *bn = BN_new(); RSA *rsa = RSA_new(); int rc; + if (!pkey || !bn || !rsa) { + if (rsa) { RSA_free(rsa); } + if (bn) { BN_free(bn); } + if (pkey){ EVP_PKEY_free(pkey); } + return NULL; + } + BN_set_word(bn, RSA_F4); rc = RSA_generate_key_ex(rsa, 2048, bn, NULL); if (rc != 1) { - RSA_free(rsa); - BN_free(bn); + RSA_free(rsa); + BN_free(bn); + EVP_PKEY_free(pkey); return NULL; }
1425-1426: Remove invalid Content-Length header additionflb_http_add_header rejects val_len=0 and client sets Content-Length.
- flb_http_add_header(client, "Content-Length", 14, NULL, 0);
1652-1656: Destroy ctx on host construction failure to avoid leakhost = construct_oci_host("loganalytics", ctx); if (!host) { flb_plg_error(ctx->ins, "failed to construct oci host"); - return NULL; + flb_oci_logan_conf_destroy(ctx); + return NULL; }
1958-1960: Fix log typo- flb_plg_info(ctx->ins, "security token expir e in %ld", exp); + flb_plg_info(ctx->ins, "security token expires at %ld", exp);
698-701: Free key_resp after extracting PEM to prevent leakkey_resp is copied into SDS; original should be destroyed.
size_t pem_len = (pem_end - pem_start) + strlen("-----END RSA PRIVATE KEY-----") + 1; ctx->imds.leaf_key = flb_sds_create_len(pem_start, pem_len); + flb_sds_destroy(key_resp);
420-453: Make region extraction robust; free tmp buffer on successHandle body‑only responses and release ‘region’.
- const char *body_start = strstr(response, "\r\n\r\n"); - if (!body_start) { - return NULL; - } - body_start += 4; + const char *body_start = strstr(response, "\r\n\r\n"); + if (body_start) { body_start += 4; } + else { body_start = response; } @@ - flb_sds_t lregion = flb_sds_create(region_value); + flb_sds_t lregion = flb_sds_create(region_value); if (!lregion) { free(region); return NULL; } - return lregion; + free(region); + return lregion;
1838-1847: Free proxy_host in destructorPrevent leak when proxy is configured.
if (ctx->u) { flb_upstream_destroy(ctx->u); } + if (ctx->proxy_host) { + flb_free(ctx->proxy_host); + }plugins/out_oracle_log_analytics/oci_logan.c (6)
556-563: Fix retry log conditionretry_error returns FLB_TRUE/FLB_FALSE, not FLB_RETRY.
- if (ret == FLB_RETRY) { + if (ret == FLB_TRUE) { flb_plg_error(ctx->ins, "HTTP %d: will retry", c->resp.status); } else { flb_plg_error(ctx->ins, "HTTP %d: non retryable error", c->resp.status); }
304-311: Sign and set Content-Type as application/jsonThe body is JSON; align signed header and request header.
- signing_str = add_header_and_signing(c, signing_str, - FLB_OCI_HEADER_CONTENT_TYPE, - sizeof(FLB_OCI_HEADER_CONTENT_TYPE) - 1, FLB_OCI_HEADER_CONTENT_TYPE_VAL, - sizeof(FLB_OCI_HEADER_CONTENT_TYPE_VAL) - 1); + signing_str = add_header_and_signing(c, signing_str, + FLB_OCI_HEADER_CONTENT_TYPE, + sizeof(FLB_OCI_HEADER_CONTENT_TYPE) - 1, + "application/json", strlen("application/json"));
739-743: Use application/json in token signing pathJSON payload requires application/json.
- flb_sds_printf(&string_to_sign, - "content-type: application/octet-stream\n"); + flb_sds_printf(&string_to_sign, + "content-type: application/json\n");
934-939: Set Content-Type application/json for JSON uploadsAvoid mismatched media type.
- if (payload && payload_size > 0) { - flb_http_add_header(client, "Content-Type", 12, - "application/octet-stream", 24); - } + if (payload && payload_size > 0) { + flb_http_add_header(client, "Content-Type", 12, + "application/json", 16); + }
2056-2059: Remove duplicate config_map entry for oci_la_log_set_idDuplicate keys cause confusion/override.
- { - FLB_CONFIG_MAP_STR, "oci_la_log_set_id", NULL, - 0, FLB_TRUE, offsetof(struct flb_oci_logan, oci_la_log_set_id), - ""},
1867-1871: Free record‑scoped log IDs after flush to avoid leaksOnly when created from record.
ret = flush_to_endpoint(ctx, out_buf, log_group_id, log_set_id); flb_sds_destroy(out_buf); - return ret; + if (ctx->oci_config_in_record == FLB_TRUE) { + if (log_group_id) { flb_sds_destroy(log_group_id); } + if (log_set_id) { flb_sds_destroy(log_set_id); } + } + return ret;
🧹 Nitpick comments (2)
plugins/out_oracle_log_analytics/oci_logan.h (2)
175-203: Remove duplicate includes (tidy‑up)Multiple includes are repeated.
#include <fluent-bit/flb_upstream.h> #include <fluent-bit/flb_sds.h> #include <fluent-bit/flb_record_accessor.h> -#include <fluent-bit/flb_hash_table.h> -#include <fluent-bit/flb_output_plugin.h> -#include <fluent-bit/flb_upstream.h> -#include <fluent-bit/flb_upstream_conn.h> -#include <fluent-bit/flb_http_client.h> -#include <fluent-bit/flb_log_event_decoder.h> -#include <fluent-bit/flb_hash_table.h> -#include <fluent-bit/flb_pack.h> -#include <fluent-bit/flb_crypto.h> -#include <fluent-bit/flb_base64.h> -#include <fluent-bit/flb_hash.h> -#include <fluent-bit/flb_sds.h> +#include <fluent-bit/flb_hash_table.h> +#include <fluent-bit/flb_output_plugin.h> +#include <fluent-bit/flb_upstream_conn.h> +#include <fluent-bit/flb_http_client.h> +#include <fluent-bit/flb_log_event_decoder.h> +#include <fluent-bit/flb_pack.h> +#include <fluent-bit/flb_crypto.h> +#include <fluent-bit/flb_base64.h> +#include <fluent-bit/flb_hash.h>
218-237: Unify IMDS field ownership (flb_sds_t vs malloc) to avoid future misusesession_pubkey/session_privkey are freed with free() elsewhere; they shouldn’t be typed as flb_sds_t.
Consider changing those to char* (or convert to SDS everywhere) and align alloc/free consistently across getters and destructor.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
plugins/out_oracle_log_analytics/CMakeLists.txt(1 hunks)plugins/out_oracle_log_analytics/oci_logan.c(33 hunks)plugins/out_oracle_log_analytics/oci_logan.h(5 hunks)plugins/out_oracle_log_analytics/oci_logan_conf.c(6 hunks)plugins/out_oracle_log_analytics/oci_logan_conf.h(1 hunks)plugins/out_oracle_log_analytics/oci_logan_helper.c(1 hunks)tests/runtime/CMakeLists.txt(1 hunks)tests/runtime/out_oracle_log_analytics.c(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/runtime/CMakeLists.txt
🧰 Additional context used
🧬 Code graph analysis (6)
plugins/out_oracle_log_analytics/oci_logan_helper.c (1)
src/flb_hash_table.c (4)
flb_hash_table_create(99-137)flb_hash_table_add(401-494)flb_hash_table_destroy(197-215)flb_hash_table_get(496-522)
plugins/out_oracle_log_analytics/oci_logan_conf.h (1)
plugins/out_oracle_log_analytics/oci_logan_conf.c (2)
create_federation_payload(898-937)sign_and_send_federation_request(1344-1505)
plugins/out_oracle_log_analytics/oci_logan_conf.c (9)
plugins/out_oracle_log_analytics/oci_logan.c (2)
EVP_MD_CTX_new(27-30)EVP_MD_CTX_free(32-35)include/fluent-bit/flb_mem.h (2)
flb_free(126-128)flb_calloc(84-96)src/flb_sds.c (6)
flb_sds_destroy(389-399)flb_sds_create_len(58-76)flb_sds_create_size(92-95)flb_sds_snprintf(405-428)flb_sds_create(78-90)flb_sds_printf(336-387)src/flb_http_client.c (4)
flb_http_client(814-859)flb_http_add_header(963-995)flb_http_do(1572-1632)flb_http_client_destroy(1688-1695)plugins/out_oracle_log_analytics/oci_logan_helper.c (3)
determine_realm_from_region(533-545)get_domain_suffix_for_realm(548-561)long_region_name(565-577)src/flb_upstream.c (4)
flb_upstream_create(290-363)flb_upstream_conn_get(711-844)flb_upstream_conn_release(862-947)flb_upstream_destroy(656-698)include/fluent-bit/flb_output.h (1)
flb_output_config_map_set(1298-1321)src/flb_output.c (2)
flb_output_net_default(1059-1069)flb_output_upstream_set(1556-1647)src/flb_utils.c (1)
flb_utils_url_split(1441-1534)
plugins/out_oracle_log_analytics/oci_logan.c (8)
plugins/out_oracle_log_analytics/oci_logan_conf.c (4)
EVP_MD_CTX_new(43-46)EVP_MD_CTX_free(48-51)create_federation_payload(898-937)sign_and_send_federation_request(1344-1505)src/flb_sds.c (6)
flb_sds_create_size(92-95)flb_sds_cat_safe(204-214)flb_sds_destroy(389-399)flb_sds_create(78-90)flb_sds_printf(336-387)flb_sds_create_len(58-76)src/flb_http_client.c (5)
flb_http_add_header(963-995)flb_http_client(814-859)flb_http_client_destroy(1688-1695)flb_http_allow_duplicated_headers(99-107)flb_http_buffer_size(872-883)src/flb_upstream.c (1)
flb_upstream_conn_get(711-844)plugins/out_oracle_log_analytics/oci_logan_helper.c (1)
is_valid_timezone(520-530)src/flb_log_event_decoder.c (2)
flb_log_event_decoder_init(99-116)flb_log_event_decoder_destroy(147-179)src/flb_pack.c (1)
flb_msgpack_raw_to_json_sds(1026-1085)src/flb_mp.c (1)
flb_mp_count(43-46)
plugins/out_oracle_log_analytics/oci_logan.h (1)
plugins/out_oracle_log_analytics/oci_logan_helper.c (4)
is_valid_timezone(520-530)get_domain_suffix_for_realm(548-561)determine_realm_from_region(533-545)long_region_name(565-577)
tests/runtime/out_oracle_log_analytics.c (1)
src/flb_lib.c (9)
flb_create(138-220)flb_service_set(647-673)flb_input(261-271)flb_destroy(223-258)flb_input_set(300-330)flb_output(274-284)flb_output_set(515-546)flb_start(914-925)flb_stop(942-985)
🪛 Gitleaks (8.28.0)
tests/runtime/out_oracle_log_analytics.c
[high] 132-158: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.
(private-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (24)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_ARROW=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
- GitHub Check: PR - fuzzing test
🔇 Additional comments (2)
plugins/out_oracle_log_analytics/oci_logan_conf.h (1)
35-37: Prototypes look goodNew federation APIs are well-scoped and const‑correct. No issues.
plugins/out_oracle_log_analytics/CMakeLists.txt (1)
2-5: Add helper to build: LGTMIncluding oci_logan_helper.c is correct.
edsiper
left a comment
edsiper
left a comment
There was a problem hiding this comment.
please adjust the code style https://github.com/fluent/fluent-bit/blob/master/CONTRIBUTING.md#coding-style (not only for my comments but in general)
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (15)
plugins/out_oracle_log_analytics/oci_logan.c (6)
553-560: Fix retry logging condition.retry_error() returns FLB_TRUE/FLB_FALSE, not FLB_RETRY.
Apply:
- if (ret == FLB_RETRY) { + if (ret == FLB_TRUE) { flb_plg_error(ctx->ins, "HTTP %d: will retry", c->resp.status); } else { flb_plg_error(ctx->ins, "HTTP %d: non retryable error", c->resp.status); }
938-943: Send Content-Type: application/json.The request body is JSON, not octet-stream.
Apply:
- if (payload && payload_size > 0) { - flb_http_add_header(client, "Content-Type", 12, - "application/octet-stream", 24); - } + if (payload && payload_size > 0) { + flb_http_add_header(client, "Content-Type", 12, + "application/json", 16); + }
301-307: Set Content-Type to application/json for JSON uploads.Body is JSON; signing/header must match.
Apply:
- signing_str = add_header_and_signing(c, signing_str, - FLB_OCI_HEADER_CONTENT_TYPE, - sizeof(FLB_OCI_HEADER_CONTENT_TYPE) - 1, FLB_OCI_HEADER_CONTENT_TYPE_VAL, - sizeof(FLB_OCI_HEADER_CONTENT_TYPE_VAL) - 1); + signing_str = add_header_and_signing(c, signing_str, + FLB_OCI_HEADER_CONTENT_TYPE, + sizeof(FLB_OCI_HEADER_CONTENT_TYPE) - 1, + "application/json", + strlen("application/json"));
742-746: Use application/json in string-to-sign for token path.Content-Type must match actual request.
Apply:
- flb_sds_printf(&string_to_sign, - "content-type: application/octet-stream\n"); + flb_sds_printf(&string_to_sign, + "content-type: application/json\n");
1726-1882: Free record-scoped log IDs when oci_config_in_record=true.log_group_id/log_set_id created from records are never destroyed in send_batch_with_count error/success paths.
Apply:
@@ - if (ret != 0) { + if (ret != 0) { flb_log_event_decoder_destroy(&log_decoder); msgpack_sbuffer_destroy(&mp_sbuf); + if (ctx->oci_config_in_record == FLB_TRUE) { + if (log_group_id) { flb_sds_destroy(log_group_id); } + if (log_set_id) { flb_sds_destroy(log_set_id); } + } return FLB_ERROR; } @@ - ret = flush_to_endpoint(ctx, out_buf, log_group_id, log_set_id); + ret = flush_to_endpoint(ctx, out_buf, log_group_id, log_set_id); flb_sds_destroy(out_buf); - return ret; + if (ctx->oci_config_in_record == FLB_TRUE) { + if (log_group_id) { flb_sds_destroy(log_group_id); } + if (log_set_id) { flb_sds_destroy(log_set_id); } + } + return ret;
988-1029: Fix hash prefix: hex-encode raw SHA-256 digest, not base64 text.Current code hexes base64 chars, producing wrong filenames.
Apply:
- char *content_sha256; + unsigned char digest[SHA256_DIGEST_LENGTH]; @@ - payload_size = flb_sds_len(payload); - content_sha256 = calculate_content_sha256_b64(payload, payload_size); - if (!content_sha256) { - return; - } + payload_size = flb_sds_len(payload); + SHA256((unsigned char *) payload, payload_size, digest); @@ - for (i = 0; i < SHA256_DIGEST_LENGTH; i++) { - sprintf(hash_in_hex + (i * 2), "%02x", content_sha256[i]); - } + for (i = 0; i < SHA256_DIGEST_LENGTH; i++) { + sprintf(hash_in_hex + (i * 2), "%02x", digest[i]); + } @@ - flb_free(content_sha256); return; @@ - flb_plg_error(ctx->ins, "cant open file -> %s", filename); - flb_free(content_sha256); + flb_plg_error(ctx->ins, "cant open file -> %s", filename); return; @@ - flb_free(content_sha256);plugins/out_oracle_log_analytics/oci_logan_conf.c (9)
766-774: Error cleanup must free partially assigned IMDS fields (not only NULL them).Current code leaks on failure paths.
Apply:
- ctx->imds.fingerprint = NULL; - ctx->imds.intermediate_cert = NULL; - ctx->imds.leaf_cert = NULL; - ctx->imds.leaf_key = NULL; - ctx->imds.region = NULL; + if (ctx->imds.fingerprint) { + flb_sds_destroy(ctx->imds.fingerprint); + ctx->imds.fingerprint = NULL; + } + if (ctx->imds.intermediate_cert) { + flb_sds_destroy(ctx->imds.intermediate_cert); + ctx->imds.intermediate_cert = NULL; + } + if (ctx->imds.leaf_cert) { + flb_free(ctx->imds.leaf_cert); + ctx->imds.leaf_cert = NULL; + } + if (ctx->imds.leaf_key) { + flb_sds_destroy(ctx->imds.leaf_key); + ctx->imds.leaf_key = NULL; + } + if (ctx->imds.region) { + flb_sds_destroy(ctx->imds.region); + ctx->imds.region = NULL; + }
2015-2017: Fix typo in log message.“expir e” -> “expires at”.
Apply:
- flb_plg_info(ctx->ins, "security token expir e in %ld", exp); + flb_plg_info(ctx->ins, "security token expires at %ld", exp);
164-169: Free SDS on empty config file content to avoid leak.content is not destroyed when length is 0.
Apply:
- if (content == NULL || flb_sds_len(content) == 0) { - return -1; - } + if (content == NULL || flb_sds_len(content) == 0) { + if (content) { flb_sds_destroy(content); } + return -1; + }
1703-1707: Free ctx on host construction failure to avoid leak.On failure you return without destroying ctx.
Apply:
- if (!host) { - flb_plg_error(ctx->ins, "failed to construct oci host"); - return NULL; - } + if (!host) { + flb_plg_error(ctx->ins, "failed to construct oci host"); + flb_oci_logan_conf_destroy(ctx); + return NULL; + }
692-731: Leak: key_resp not destroyed on success path.After extracting the PEM block into ctx->imds.leaf_key, free key_resp.
Apply:
pem_len = (pem_end - pem_start) + strlen("-----END RSA PRIVATE KEY-----") + 1; ctx->imds.leaf_key = flb_sds_create_len(pem_start, pem_len); + flb_sds_destroy(key_resp);
781-801: OpenSSL<3: check allocations and free on failure (pkey/bn/rsa).Avoid NULL deref and leaks.
Apply:
#if OPENSSL_VERSION_NUMBER < 0x30000000L EVP_PKEY *pkey; BIGNUM *bn; RSA *rsa; int rc; pkey = EVP_PKEY_new(); bn = BN_new(); rsa = RSA_new(); + if (!pkey || !bn || !rsa) { + if (rsa) { RSA_free(rsa); } + if (bn) { BN_free(bn); } + if (pkey){ EVP_PKEY_free(pkey); } + return NULL; + } BN_set_word(bn, RSA_F4); rc = RSA_generate_key_ex(rsa, 2048, bn, NULL); if (rc != 1) { RSA_free(rsa); BN_free(bn); + EVP_PKEY_free(pkey); return NULL; } EVP_PKEY_assign_RSA(pkey, rsa); BN_free(bn); return pkey;
430-463: extract_region: handle headerless bodies and free temporary buffer on success.
- Treat full input as body when headers are absent.
- Free region before returning to prevent a leak.
Apply:
- body_start = strstr(response, "\r\n\r\n"); - if (!body_start) { - return NULL; - } - body_start += 4; + body_start = strstr(response, "\r\n\r\n"); + if (body_start) { + body_start += 4; + } + else { + /* No headers; treat entire input as body */ + body_start = response; + } @@ - lregion = flb_sds_create(region_value); - if (!lregion) { - free(region); - return NULL; - } - return lregion; + lregion = flb_sds_create(region_value); + if (!lregion) { + free(region); + return NULL; + } + free(region); + return lregion;
1828-1900: Complete destructor: free proxy_host as well.proxy_host allocated during proxy parsing isn’t released.
Apply:
if (ctx->u) { flb_upstream_destroy(ctx->u); } + if (ctx->proxy_host) { + flb_free(ctx->proxy_host); + }
1479-1480: Remove invalid manual Content-Length header.HTTP client sets Content-Length; adding with NULL value is rejected.
Apply:
- flb_http_add_header(client, "Content-Length", 14, NULL, 0);
🧹 Nitpick comments (2)
plugins/out_oracle_log_analytics/oci_logan_conf.c (1)
1934-1941: Gitleaks false-positive: obfuscate mock private key or guard with test-only macro.Hardcoded PEM markers trigger secret scanners. Wrap/test-gate or split the marker to avoid alerts.
Example obfuscation:
- return flb_sds_create("-----BEGIN RSA PRIVATE KEY-----\n" + return flb_sds_create("-----BEGIN RSA PRIV" "ATE KEY-----\n" @@ - "-----END RSA PRIVATE KEY-----"); + "-----END RSA PRIV" "ATE KEY-----");Alternatively, wrap mock_imds_request under a compile-time test guard (e.g., #ifdef FLB_HAVE_TESTS) or move fixtures into test files. The scanner flagged these regions (private-key).
plugins/out_oracle_log_analytics/oci_logan.c (1)
1768-1786: Consider exact-match between signed path and actual URI.You sign using an encoded URI but send the unencoded one. If ctx->uri or params ever need encoding, signatures can fail. Ensure both use the same encoded path.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
plugins/out_oracle_log_analytics/oci_logan.c(32 hunks)plugins/out_oracle_log_analytics/oci_logan_conf.c(6 hunks)plugins/out_oracle_log_analytics/oci_logan_helper.c(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
plugins/out_oracle_log_analytics/oci_logan_helper.c (1)
src/flb_hash_table.c (4)
flb_hash_table_create(99-137)flb_hash_table_add(401-494)flb_hash_table_destroy(197-215)flb_hash_table_get(496-522)
plugins/out_oracle_log_analytics/oci_logan_conf.c (9)
plugins/out_oracle_log_analytics/oci_logan.c (2)
EVP_MD_CTX_new(27-30)EVP_MD_CTX_free(32-35)include/fluent-bit/flb_mem.h (2)
flb_free(126-128)flb_calloc(84-96)src/flb_sds.c (6)
flb_sds_destroy(389-399)flb_sds_create_len(58-76)flb_sds_create_size(92-95)flb_sds_snprintf(405-428)flb_sds_create(78-90)flb_sds_printf(336-387)src/flb_http_client.c (4)
flb_http_client(814-859)flb_http_add_header(963-995)flb_http_do(1572-1632)flb_http_client_destroy(1688-1695)plugins/out_oracle_log_analytics/oci_logan_helper.c (3)
determine_realm_from_region(533-546)get_domain_suffix_for_realm(549-563)long_region_name(567-580)src/flb_upstream.c (4)
flb_upstream_create(290-363)flb_upstream_conn_get(711-844)flb_upstream_conn_release(862-947)flb_upstream_destroy(656-698)include/fluent-bit/flb_output.h (1)
flb_output_config_map_set(1298-1321)src/flb_output.c (2)
flb_output_net_default(1059-1069)flb_output_upstream_set(1556-1647)src/flb_utils.c (1)
flb_utils_url_split(1441-1534)
plugins/out_oracle_log_analytics/oci_logan.c (9)
plugins/out_oracle_log_analytics/oci_logan_conf.c (4)
EVP_MD_CTX_new(43-46)EVP_MD_CTX_free(48-51)create_federation_payload(941-984)sign_and_send_federation_request(1396-1556)src/flb_sds.c (6)
flb_sds_create_size(92-95)flb_sds_cat_safe(204-214)flb_sds_destroy(389-399)flb_sds_create(78-90)flb_sds_printf(336-387)flb_sds_create_len(58-76)src/flb_http_client.c (5)
flb_http_add_header(963-995)flb_http_client(814-859)flb_http_client_destroy(1688-1695)flb_http_allow_duplicated_headers(99-107)flb_http_buffer_size(872-883)include/fluent-bit/flb_mem.h (1)
flb_free(126-128)src/flb_upstream.c (1)
flb_upstream_conn_get(711-844)plugins/out_oracle_log_analytics/oci_logan_helper.c (1)
is_valid_timezone(520-530)src/flb_log_event_decoder.c (3)
flb_log_event_decoder_init(99-116)flb_log_event_decoder_next(310-406)flb_log_event_decoder_destroy(147-179)src/flb_pack.c (1)
flb_msgpack_raw_to_json_sds(1026-1085)src/flb_mp.c (1)
flb_mp_count(43-46)
🪛 Gitleaks (8.28.0)
plugins/out_oracle_log_analytics/oci_logan_conf.c
[high] 653-726: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.
(private-key)
[high] 1934-1940: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.
(private-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (80)
- GitHub Check: PR - container builds / Windows container images (2025)
- GitHub Check: PR - container builds / Windows container images (2022)
- GitHub Check: PR - packages build Linux / ubuntu/22.04 package build and stage to S3
- GitHub Check: PR - packages build Linux / debian/bookworm.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / ubuntu/24.04 package build and stage to S3
- GitHub Check: PR - packages build Linux / ubuntu/24.04.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / debian/bookworm package build and stage to S3
- GitHub Check: PR - packages build Linux / raspbian/bookworm package build and stage to S3
- GitHub Check: PR - packages build Linux / debian/trixie.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / debian/trixie package build and stage to S3
- GitHub Check: PR - packages build Linux / rockylinux/8.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / ubuntu/22.04.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / almalinux/10.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / debian/buster package build and stage to S3
- GitHub Check: PR - packages build Linux / almalinux/10 package build and stage to S3
- GitHub Check: PR - packages build Linux / rockylinux/10.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / centos/10.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / centos/7 package build and stage to S3
- GitHub Check: PR - packages build Linux / rockylinux/8 package build and stage to S3
- GitHub Check: PR - packages build Linux / debian/buster.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / debian/bullseye package build and stage to S3
- GitHub Check: PR - packages build Linux / rockylinux/10 package build and stage to S3
- GitHub Check: PR - packages build Linux / almalinux/8.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / almalinux/9 package build and stage to S3
- GitHub Check: PR - packages build Linux / centos/9 package build and stage to S3
- GitHub Check: PR - packages build Linux / rockylinux/9 package build and stage to S3
- GitHub Check: PR - packages build Linux / debian/bullseye.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / almalinux/8 package build and stage to S3
- GitHub Check: PR - packages build Linux / rockylinux/9.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / almalinux/9.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / centos/7.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / centos/10 package build and stage to S3
- GitHub Check: PR - packages build Linux / centos/9.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / amazonlinux/2023 package build and stage to S3
- GitHub Check: PR - packages build Linux / amazonlinux/2023.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / centos/8 package build and stage to S3
- GitHub Check: PR - packages build Linux / centos/8.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / amazonlinux/2 package build and stage to S3
- GitHub Check: PR - packages build Linux / amazonlinux/2.arm64v8 package build and stage to S3
- GitHub Check: PR - container builds / amd64/production container image build
- GitHub Check: PR - container builds / arm/v7/debug container image build
- GitHub Check: PR - container builds / arm64/production container image build
- GitHub Check: PR - container builds / amd64/debug container image build
- GitHub Check: PR - container builds / arm/v7/production container image build
- GitHub Check: PR - container builds / arm64/debug container image build
- GitHub Check: PR - packages build Windows / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
- GitHub Check: PR - packages build Windows / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
- GitHub Check: PR - packages build Windows / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
- GitHub Check: PR - packages build MacOS / call-build-macos-package (Apple Silicon macOS runner, macos-14, 3.31.6)
- GitHub Check: PR - packages build MacOS / call-build-macos-package (Intel macOS runner, macos-14-large, 3.31.6)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_ARROW=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: PR - fuzzing test
- GitHub Check: pr-compile-centos-7
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-without-cxx (3.31.6)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (13)
plugins/out_oracle_log_analytics/oci_logan.h (1)
218-230: Fix IMDS field type inconsistency to prevent crashes.Fields
leaf_cert,session_pubkey, andsession_privkeyare declared asflb_sds_tbut the destructor (oci_logan_conf.c lines 1839-1851) frees them with plainfree()instead offlb_sds_destroy(), and some code paths assign malloc'd strings. This type mismatch can cause crashes or leaks.Unify the type to plain
char*throughout, or ensure all allocations use SDS APIs and all frees useflb_sds_destroy(). For example:struct flb_oracle_imds { flb_sds_t region; - flb_sds_t leaf_cert; + char *leaf_cert; flb_sds_t leaf_key; flb_sds_t intermediate_cert; flb_sds_t tenancy_ocid; flb_sds_t fingerprint; - flb_sds_t session_pubkey; - flb_sds_t session_privkey; + char *session_pubkey; + char *session_privkey; struct flb_upstream *upstream; struct flb_output_instance *ins; };Then update the destructor in oci_logan_conf.c to consistently use
flb_free()for these fields.Based on learnings
tests/runtime/out_oracle_log_analytics.c (2)
74-75: Unset FLB_OCI_PLUGIN_UNDER_TEST to prevent test isolation issues.
FLB_OCI_PLUGIN_UNDER_TESTis set on line 21 but never unset, which can leak state into subsequent tests.Apply this diff:
flb_destroy(ctx); + unsetenv("FLB_OCI_PLUGIN_UNDER_TEST"); unsetenv("TEST_IMDS_SUCCESS"); }
131-158: Replace hardcoded private key with non-sensitive placeholder.The literal RSA private key triggers secret scanners and is unnecessary for testing. Replace with a clearly fake placeholder or load from a test fixture.
Apply this diff:
- char key_content[] = - "-----BEGIN RSA PRIVATE KEY-----\n" - "MIIEowIBAAKCAQEAy8Dbv8prpJ/0kKhlGeJYozo2t60EG8L0561g13R29LvMR5hy\n" - ... - "VDLq3cL2MQKmVPrmWCFKLJSXiKGmqYZmVXC7FqfJJrKqLdFQCZNf\n" - "-----END RSA PRIVATE KEY-----\n"; + /* Non-sensitive test key placeholder */ + char key_content[] = + "-----BEGIN RSA PRIVATE KEY-----\n" + "TEST_KEY_PLACEHOLDER_DO_NOT_USE_IN_PRODUCTION\n" + "-----END RSA PRIVATE KEY-----\n";plugins/out_oracle_log_analytics/oci_logan_helper.c (1)
445-474: Make lazy timezone hash initialization thread-safe.Concurrent first-calls to
init_oci_timezone_hash()can race, causing memory leaks or double-destruction. Add double-checked locking with a mutex.Apply this diff:
+#include <pthread.h> + +static pthread_mutex_t oci_tz_lock = PTHREAD_MUTEX_INITIALIZER; + static int init_oci_timezone_hash(void) { int i; int ret; if (oci_timezone_hash != NULL) { return 0; } + + pthread_mutex_lock(&oci_tz_lock); + if (oci_timezone_hash != NULL) { + pthread_mutex_unlock(&oci_tz_lock); + return 0; + } oci_timezone_hash = flb_hash_table_create(FLB_HASH_TABLE_EVICT_NONE, 1024, -1); if (!oci_timezone_hash) { + pthread_mutex_unlock(&oci_tz_lock); return -2; } for (i = 0; oci_supported_timezones[i] != NULL; i++) { ret = flb_hash_table_add(oci_timezone_hash, oci_supported_timezones[i], strlen(oci_supported_timezones[i]), (void *) "1", sizeof("1")); if (ret < 0) { flb_hash_table_destroy(oci_timezone_hash); oci_timezone_hash = NULL; + pthread_mutex_unlock(&oci_tz_lock); return -3; } } + pthread_mutex_unlock(&oci_tz_lock); return 0; }plugins/out_oracle_log_analytics/oci_logan_conf.c (3)
421-463: Free temporary region buffer and handle body-only responses.The
regionbuffer allocated at line 448 is never freed on the success path, causing a memory leak. Additionally, the function doesn't handle responses without headers (body-only).Apply this diff:
flb_sds_t extract_region(const char *response) { const char *body_start; size_t len; char *region; const char *long_name; const char *region_value; flb_sds_t lregion; body_start = strstr(response, "\r\n\r\n"); - if (!body_start) { - return NULL; - } - - body_start += 4; + if (body_start) { + body_start += 4; + } + else { + /* No headers present: treat the full input as body */ + body_start = response; + } while (*body_start == '\n' || *body_start == '\r' || *body_start == ' ') { body_start++; } len = strlen(body_start); while (len > 0 && (body_start[len - 1] == '\n' || body_start[len - 1] == '\r' || body_start[len - 1] == ' ')) { len--; } region = malloc(len + 1); if (!region) { return NULL; } strncpy(region, body_start, len); region[len] = '\0'; long_name = long_region_name(region); region_value = long_name ? long_name : region; lregion = flb_sds_create(region_value); if (!lregion) { free(region); return NULL; } + free(region); return lregion; }
781-800: Check OpenSSL allocation results before use (OpenSSL <3.0).Lines 787-789 allocate
pkey,bn, andrsawithout NULL checks. If any allocation fails, subsequent operations will dereference NULL pointers, causing a crash.Apply this diff:
#if OPENSSL_VERSION_NUMBER < 0x30000000L EVP_PKEY *pkey = EVP_PKEY_new(); BIGNUM *bn = BN_new(); RSA *rsa = RSA_new(); int rc; + if (!pkey || !bn || !rsa) { + if (rsa) { RSA_free(rsa); } + if (bn) { BN_free(bn); } + if (pkey){ EVP_PKEY_free(pkey); } + return NULL; + } + BN_set_word(bn, RSA_F4); rc = RSA_generate_key_ex(rsa, 2048, bn, NULL); if (rc != 1) { RSA_free(rsa); BN_free(bn); + EVP_PKEY_free(pkey); return NULL; }
1479-1479: Remove invalid Content-Length header addition.
flb_http_add_headerwithval_len=0is invalid and silently fails. The HTTP client already manages Content-Length automatically.Apply this diff:
flb_http_add_header(client, "Date", 4, date_header, flb_sds_len(date_header)); flb_http_add_header(client, "Content-Type", 12, "application/json", 16); - flb_http_add_header(client, "Content-Length", 14, NULL, 0);plugins/out_oracle_log_analytics/oci_logan.c (6)
742-746: Use application/json for JSON payload signing.Line 744 signs with
content-type: application/octet-stream, but the payload is JSON. The signed Content-Type must match the actual request header.Apply this diff:
if (payload && payload_size > 0) { flb_sds_printf(&string_to_sign, - "content-type: application/octet-stream\n"); + "content-type: application/json\n"); flb_sds_printf(&string_to_sign, "content-length: %zu", payload_size); }
2064-2066: Remove duplicate oci_la_log_set_id config_map entry.The key
oci_la_log_set_idis defined at both lines 2048-2050 and lines 2064-2066. Remove the duplicate to avoid key override.Apply this diff:
- { - FLB_CONFIG_MAP_STR, "oci_la_log_set_id", NULL, - 0, FLB_TRUE, offsetof(struct flb_oci_logan, oci_la_log_set_id), - ""},Based on learnings
553-559: Fix retry condition check.
retry_error()returnsFLB_TRUE/FLB_FALSE, notFLB_RETRY. The condition at line 553 will never match, causing incorrect logging.Apply this diff:
- if (ret == FLB_RETRY) { + if (ret == FLB_TRUE) { flb_plg_error(ctx->ins, "HTTP %d: will retry", c->resp.status); } else { flb_plg_error(ctx->ins, "HTTP %d: non retryable error", c->resp.status); }
937-940: Set Content-Type to application/json for JSON uploads.The upload body is JSON, but line 939 sets
Content-Type: application/octet-stream. This mismatches the signed header and OCI API expectations.Apply this diff:
if (payload && payload_size > 0) { flb_http_add_header(client, "Content-Type", 12, - "application/octet-stream", 24); + "application/json", 16); }
301-307: Set Content-Type to application/json in signed headers (config_file path).The upload body is JSON. Line 304 signs and sets
Content-TypeusingFLB_OCI_HEADER_CONTENT_TYPE_VAL, which is likelyapplication/octet-stream. Change toapplication/jsonto match the payload format.Apply this diff:
signing_str = add_header_and_signing(c, signing_str, FLB_OCI_HEADER_CONTENT_TYPE, - sizeof(FLB_OCI_HEADER_CONTENT_TYPE) - - 1, FLB_OCI_HEADER_CONTENT_TYPE_VAL, - sizeof - (FLB_OCI_HEADER_CONTENT_TYPE_VAL) - - 1); + sizeof(FLB_OCI_HEADER_CONTENT_TYPE) - 1, + "application/json", 16);Alternatively, update
FLB_OCI_HEADER_CONTENT_TYPE_VALto"application/json"in the header file.
981-1027: Compute hex from raw SHA‑256 digest (not base64 string).Lines 1002-1004 hex-encode the base64 string returned by
calculate_content_sha256_b64, producing incorrect hash prefixes. Compute hex from the raw 32-byte digest instead.Apply this diff:
- char *content_sha256; + unsigned char digest[SHA256_DIGEST_LENGTH]; @@ payload_size = flb_sds_len(payload); - content_sha256 = calculate_content_sha256_b64(payload, payload_size); - if (!content_sha256) { - return; - } + SHA256((unsigned char *) payload, payload_size, digest); for (i = 0; i < SHA256_DIGEST_LENGTH; i++) { - sprintf(hash_in_hex + (i * 2), "%02x", content_sha256[i]); + sprintf(hash_in_hex + (i * 2), "%02x", digest[i]); } @@ if (access(filename, F_OK) == 0) { flb_plg_debug(ctx->ins, "payload s already dumped to->%s", filename); - flb_free(content_sha256); return; } @@ fp = fopen(filename, "w"); if (!fp) { flb_plg_error(ctx->ins, "cant open file -> %s", filename); - flb_free(content_sha256); return; } fprintf(fp, "%s", payload); fclose(fp); - flb_free(content_sha256);
🧹 Nitpick comments (1)
plugins/out_oracle_log_analytics/oci_logan.h (1)
179-203: Remove duplicate includes.Several headers are included multiple times (flb_upstream.h at lines 180-181, flb_hash_table.h at lines 184-185, flb_sds.h at lines 189-190), creating unnecessary redundancy.
Apply this diff to remove duplicates:
#include <fluent-bit/flb_upstream.h> #include <fluent-bit/flb_sds.h> #include <fluent-bit/flb_record_accessor.h> #include <fluent-bit/flb_hash_table.h> #include <fluent-bit/flb_output_plugin.h> -#include <fluent-bit/flb_upstream.h> -#include <fluent-bit/flb_upstream_conn.h> -#include <fluent-bit/flb_http_client.h> -#include <fluent-bit/flb_log_event_decoder.h> -#include <fluent-bit/flb_hash_table.h> -#include <fluent-bit/flb_pack.h> -#include <fluent-bit/flb_crypto.h> -#include <fluent-bit/flb_base64.h> -#include <fluent-bit/flb_hash.h> -#include <fluent-bit/flb_sds.h> #include <monkey/mk_core/mk_list.h> #include <fluent-bit/flb_jsmn.h>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
plugins/out_oracle_log_analytics/CMakeLists.txt(1 hunks)plugins/out_oracle_log_analytics/oci_logan.c(32 hunks)plugins/out_oracle_log_analytics/oci_logan.h(5 hunks)plugins/out_oracle_log_analytics/oci_logan_conf.c(6 hunks)plugins/out_oracle_log_analytics/oci_logan_conf.h(1 hunks)plugins/out_oracle_log_analytics/oci_logan_helper.c(1 hunks)tests/runtime/CMakeLists.txt(1 hunks)tests/runtime/out_oracle_log_analytics.c(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/runtime/CMakeLists.txt
🧰 Additional context used
🧬 Code graph analysis (6)
tests/runtime/out_oracle_log_analytics.c (2)
plugins/out_s3/s3.c (1)
setenv(57-60)src/flb_lib.c (9)
flb_create(138-220)flb_service_set(647-673)flb_input(261-271)flb_destroy(223-258)flb_input_set(300-330)flb_output(274-284)flb_output_set(515-546)flb_start(914-925)flb_stop(942-985)
plugins/out_oracle_log_analytics/oci_logan_helper.c (1)
src/flb_hash_table.c (4)
flb_hash_table_create(99-137)flb_hash_table_add(401-494)flb_hash_table_destroy(197-215)flb_hash_table_get(496-522)
plugins/out_oracle_log_analytics/oci_logan_conf.c (7)
include/fluent-bit/flb_mem.h (2)
flb_free(126-128)flb_calloc(84-96)src/flb_sds.c (4)
flb_sds_create_len(58-76)flb_sds_create_size(92-95)flb_sds_create(78-90)flb_sds_printf(336-387)src/flb_http_client.c (4)
flb_http_client(814-859)flb_http_add_header(963-995)flb_http_do(1572-1632)flb_http_client_destroy(1688-1695)plugins/out_oracle_log_analytics/oci_logan_helper.c (3)
determine_realm_from_region(533-546)get_domain_suffix_for_realm(549-563)long_region_name(567-580)src/flb_upstream.c (4)
flb_upstream_create(290-363)flb_upstream_conn_get(711-844)flb_upstream_conn_release(862-947)flb_upstream_destroy(656-698)src/flb_output.c (2)
flb_output_net_default(1059-1069)flb_output_upstream_set(1556-1647)src/flb_utils.c (1)
flb_utils_url_split(1441-1534)
plugins/out_oracle_log_analytics/oci_logan.h (1)
plugins/out_oracle_log_analytics/oci_logan_helper.c (4)
is_valid_timezone(520-530)get_domain_suffix_for_realm(549-563)determine_realm_from_region(533-546)long_region_name(567-580)
plugins/out_oracle_log_analytics/oci_logan.c (6)
plugins/out_oracle_log_analytics/oci_logan_conf.c (4)
EVP_MD_CTX_new(43-46)EVP_MD_CTX_free(48-51)create_federation_payload(941-984)sign_and_send_federation_request(1396-1555)src/flb_sds.c (6)
flb_sds_create_size(92-95)flb_sds_cat_safe(204-214)flb_sds_destroy(389-399)flb_sds_create(78-90)flb_sds_printf(336-387)flb_sds_create_len(58-76)src/flb_http_client.c (5)
flb_http_add_header(963-995)flb_http_client(814-859)flb_http_client_destroy(1688-1695)flb_http_allow_duplicated_headers(99-107)flb_http_buffer_size(872-883)src/flb_upstream.c (1)
flb_upstream_conn_get(711-844)src/flb_log_event_decoder.c (3)
flb_log_event_decoder_init(99-116)flb_log_event_decoder_next(310-406)flb_log_event_decoder_destroy(147-179)src/flb_pack.c (1)
flb_msgpack_raw_to_json_sds(1026-1085)
plugins/out_oracle_log_analytics/oci_logan_conf.h (1)
plugins/out_oracle_log_analytics/oci_logan_conf.c (2)
create_federation_payload(941-984)sign_and_send_federation_request(1396-1555)
🪛 Gitleaks (8.28.0)
tests/runtime/out_oracle_log_analytics.c
[high] 132-158: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.
(private-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (80)
- GitHub Check: PR - container builds / Windows container images (2025)
- GitHub Check: PR - container builds / Windows container images (2022)
- GitHub Check: PR - packages build Linux / ubuntu/22.04 package build and stage to S3
- GitHub Check: PR - packages build Linux / almalinux/9 package build and stage to S3
- GitHub Check: PR - packages build Linux / debian/bullseye package build and stage to S3
- GitHub Check: PR - packages build Linux / ubuntu/24.04.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / almalinux/10.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / debian/buster package build and stage to S3
- GitHub Check: PR - packages build Linux / almalinux/9.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / ubuntu/24.04 package build and stage to S3
- GitHub Check: PR - packages build Linux / debian/bookworm package build and stage to S3
- GitHub Check: PR - packages build Linux / debian/trixie package build and stage to S3
- GitHub Check: PR - packages build Linux / debian/trixie.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / debian/bookworm.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / raspbian/bookworm package build and stage to S3
- GitHub Check: PR - packages build Linux / almalinux/10 package build and stage to S3
- GitHub Check: PR - packages build Linux / debian/bullseye.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / ubuntu/22.04.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / debian/buster.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / almalinux/8 package build and stage to S3
- GitHub Check: PR - packages build Linux / almalinux/8.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / rockylinux/8 package build and stage to S3
- GitHub Check: PR - packages build Linux / centos/10.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / rockylinux/9.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / centos/9 package build and stage to S3
- GitHub Check: PR - packages build Linux / centos/9.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / rockylinux/10.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / centos/10 package build and stage to S3
- GitHub Check: PR - packages build Linux / rockylinux/10 package build and stage to S3
- GitHub Check: PR - packages build Linux / rockylinux/8.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / centos/8 package build and stage to S3
- GitHub Check: PR - packages build Linux / rockylinux/9 package build and stage to S3
- GitHub Check: PR - packages build Linux / centos/7.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / centos/8.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / centos/7 package build and stage to S3
- GitHub Check: PR - packages build Linux / amazonlinux/2023.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / amazonlinux/2 package build and stage to S3
- GitHub Check: PR - packages build Linux / amazonlinux/2.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / amazonlinux/2023 package build and stage to S3
- GitHub Check: PR - container builds / arm64/debug container image build
- GitHub Check: PR - container builds / arm/v7/debug container image build
- GitHub Check: PR - container builds / arm64/production container image build
- GitHub Check: PR - container builds / arm/v7/production container image build
- GitHub Check: PR - container builds / amd64/production container image build
- GitHub Check: PR - container builds / amd64/debug container image build
- GitHub Check: PR - packages build Windows / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
- GitHub Check: PR - packages build Windows / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
- GitHub Check: PR - packages build Windows / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
- GitHub Check: PR - packages build MacOS / call-build-macos-package (Apple Silicon macOS runner, macos-14, 3.31.6)
- GitHub Check: PR - packages build MacOS / call-build-macos-package (Intel macOS runner, macos-14-large, 3.31.6)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_ARROW=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-without-cxx (3.31.6)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-centos-7
- GitHub Check: PR - fuzzing test
i updated the comment format also variable declaration style |
|
I think my comment got lost in the spam from Github: #10990 (comment) |
should be good now i updated the pr message, also i did add unit tests that cover all supported authentications methods by the plugin |
|
@patrick-stephens I've resolved all the issues raised by Coderabbit here's a summary of the fixes: Memory Management & Resource Cleanup:
Null Pointer & Safety Checks:
Logic & Error Handling:
Data Handling:
Configuration:
|
|
@edsiper |
Adds Instance Metadata Service based authentication support with federation token signing, JWT handling, log chunking for large payloads, and timezone support. Signed-off-by: rghouzra <rghouzra@student.1337.ma> Signed-off-by: dexter <rghouzra@student.1337.ma>
Signed-off-by: rghouzra <rghouzra@student.1337.ma> Signed-off-by: dexter <rghouzra@student.1337.ma>
Signed-off-by: Reda Ghouzraf <rghouzra@student.1337.ma>
Signed-off-by: Reda Ghouzraf <reda.ghouzraf@oracle.com>
3 participants
Problem
Oracle cloud infrastructure users running Fluent Bit on oci compute instances need a way to authenticate and send logs to oracle log analytics without managing api keys manually. Instance principal authentication provides a secure, automated way to authenticate using the instance's identity.
Change
This PR adds oci Instance Principal authentication support to the
out_oracle_log_analyticsplugin:Key Configuration Options Added:
auth_type: Choose betweenconfig_fileorinstance_principalauths, also by if the user didn't specify it, will be by default config_file>domain_suffix: Optional OCI domain suffix for different realmsoci_la_timezone: Timezone configuration for log timestamps (the supported timzeones by oci log analytics)dump_payload_file/payload_file_location: Debug options to dump payloads locallyTesting
ok-package-testlabel to test for all targetsexample configuration file:
Instance Principal Authentication: