Skip to content

aws_credentials: Add Greengrass V2 IoT provider support#11470

Open
SagiROosto wants to merge 1 commit intofluent:masterfrom
AnyVisionltd:add-aws-iot-credentials
Open

aws_credentials: Add Greengrass V2 IoT provider support#11470
SagiROosto wants to merge 1 commit intofluent:masterfrom
AnyVisionltd:add-aws-iot-credentials

Conversation

@SagiROosto
Copy link

@SagiROosto SagiROosto commented Feb 16, 2026

This update adds support for AWS authentication using AWS IOT Greengrass V2 style.
This authentication type rely on mTLS-encrypted HTTPS request to specific dedicated AWS endpoint with specific role alias (usually tokenExchangeRole) and thingName. These config are stored inside the root installation dir at .../greengrass/config/config,yaml. So technically everything needed to do is:

  1. parse the config file
  2. do HTTPS request to get temporary credentials
  3. parse the response to the known AWS format.

The cURL equivalent of:

curl -s \
  --cert "$CERT_PATH" \
  --key "$KEY_PATH" \
  --cacert "$CA_PATH" \
  -H "x-amzn-iot-thingname: $THING_NAME" \
  "https://${CRED_ENDPOINT}/role-aliases/${ROLE_ALIAS}/credentials" | jq -r '{
    "Version": 1,
    "AccessKeyId": .credentials.accessKeyId,
    "SecretAccessKey": .credentials.secretAccessKey,
    "SessionToken": .credentials.sessionToken,
    "Expiration": .credentials.expiration
  }'

*Technically credentials_process in aws config can do the trick, but that means fluent-bit must run as systemd or with shell if credentials_process script is (ba)sh, or pre-compiled and attached binary on docker container.

To use this type of aws authentication, users can do one of the following:

  1. Set environment variable AWS_IOT_GREENGRASS_V2_CONFIG_PATH to point directly to the greengrass config file
  2. Set values environment variables AWS_IOT_... like AWS_IOT_KEY_FILE, AWS_IOT_THING_NAME, etc...
    Optionally: set AWS_GG_ROOT_CA_PATH env var to point to the certificates path if stored in another directory then whats listed in config (e.g. mounted to another directory in docker container

Enter [N/A] in the box, if an item is not applicable to your change.

Testing
Before we can approve your change; please submit the following in a comment:

  • [N/A] Example configuration file for the change
  • Debug log output from testing the change
  • Attached Valgrind output that shows no leaks or memory corruption was found

If this is a change to packaging of containers or native binaries then please confirm it works for all targets.

  • Run local packaging test showing all targets (including any new ones) build.
  • Set ok-package-test label to test for all targets (requires maintainer to do).

Documentation

  • Documentation required for this feature

Backporting

  • Backport to latest stable release.

Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.

Summary by CodeRabbit

  • New Features

    • Added an AWS IoT credential provider to the authentication chain (Environment → IoT → Shared/Profile → ...).
    • Support for IoT certificate/key/CA, credentials endpoint, thing name, role alias, and Greengrass V2 config/root CA for credential discovery.
  • Documentation

    • Updated provider ordering and IoT configuration notes.

@coderabbitai
Copy link

coderabbitai bot commented Feb 16, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds a new IoT-based AWS credentials provider (implementation, public macros, and creation function), integrates it into the credentials provider chain, and updates build sources and a header year range.

Changes

Cohort / File(s) Summary
Public API Declarations
include/fluent-bit/flb_aws_credentials.h
Added IoT-related macros (AWS_IOT_KEY_FILE, AWS_IOT_CERT_FILE, AWS_IOT_CA_CERT_FILE, AWS_IOT_CREDENTIALS_ENDPOINT, AWS_IOT_THING_NAME, AWS_IOT_ROLE_ALIAS, AWS_IOT_GREENGRASS_V2_CONFIG, AWS_GG_ROOT_CA_PATH) and declaration for flb_iot_provider_create().
Build Configuration
src/aws/CMakeLists.txt
Added flb_aws_credentials_iot.c to the flb-aws static library sources.
IoT Credentials Provider Implementation
src/aws/flb_aws_credentials_iot.c
New IoT provider implementation: provider struct and lifecycle (create/init/refresh/get/sync/async/upstream_set/destroy), Greengrass v2 YAML parsing (optional), TLS/upstream setup, HTTP client, IoT credentials request/response flow, JSON parsing, and cleanup/error handling.
Provider Chain Integration
src/aws/flb_aws_credentials.c
Inserted IoT provider into the credentials provider chain (after environment provider, before shared/profile provider) with initialization and logging; updated chain ordering in comments.
Maintenance
src/aws/flb_aws_credentials_log.h
Updated license/header year range (comment-only change).

Sequence Diagram

sequenceDiagram
    participant Config as Fluent Bit Config
    participant IoTProv as IoT Provider
    participant TLS as TLS Context
    participant Upstream as Upstream/TLS Conn
    participant HTTP as HTTP Client
    participant IoTEndpoint as IoT Credentials Endpoint
    participant Parser as JSON Parser

    Config->>IoTProv: flb_iot_provider_create(config, generator)
    IoTProv->>IoTProv: read env / parse Greengrass v2 config
    IoTProv->>TLS: build TLS context (cert/key/CA)
    TLS-->>IoTProv: TLS context ready
    IoTProv->>Upstream: create upstream with TLS
    Upstream-->>IoTProv: upstream ready
    IoTProv->>HTTP: create HTTP client (headers incl. thing name)
    Config->>IoTProv: refresh / get_credentials
    IoTProv->>IoTEndpoint: HTTPS GET /role-aliases/.../credentials
    IoTEndpoint-->>HTTP: JSON response
    HTTP->>Parser: deliver body
    Parser-->>IoTProv: AccessKeyId, SecretAccessKey, Token, Expiration
    IoTProv-->>Config: provide flb_aws_credentials
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested reviewers

  • koleini

Poem

🐇 I nibble certs by moonlit logs,

I parse Greengrass whispers through the fog,
Role-alias roads and HTTPS streams,
Keys hop out of careful dreams,
Hooray — credentials bound for beams!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 15.38% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main addition: AWS IoT provider support for Greengrass V2 credentials, which matches the primary objective of the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 19a31f463d

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

🤖 Fix all issues with AI agents
In `@src/aws/flb_aws_credentials_iot.c`:
- Around line 765-769: The debug log AWS_CREDS_DEBUG currently prints the full
IoT credentials response via c->resp.payload and c->resp.payload_size which can
expose SecretAccessKey and SessionToken; remove this full-payload log or replace
it with a redacted summary (e.g., log only payload size and a masked/omitted
credentials indicator) so that sensitive fields from c->resp.payload are never
output; locate the AWS_CREDS_DEBUG call referencing
c->resp.payload/c->resp.payload_size and change it to redact or omit secrets
before logging.
- Around line 679-681: The header length is wrong:
implementation->thing_name_header.key_len is set to 22 while the string
"x-amzn-iot-thingname" is 20 chars, causing two garbage bytes to be included;
fix by setting thing_name_header.key_len to the correct length (e.g., compute
with strlen("x-amzn-iot-thingname") or use sizeof("x-amzn-iot-thingname") - 1)
where implementation->thing_name_header.key is assigned so the HTTP header uses
the exact byte count.
- Around line 562-575: The allocation failure paths for provider and
implementation (flb_calloc calls creating provider and implementation) currently
return early and do not free gg_cfg when gg_parsed is true; modify the function
so allocation failures route to the same cleanup that frees gg_cfg (e.g., set
provider = NULL and goto cleanup or call the existing cleanup helper) instead of
returning directly, ensuring gg_cfg is freed when gg_parsed is set and that
provider/implementation are freed/null-checked appropriately before final
return.
- Around line 656-662: The NULL return from generator->create() currently calls
flb_aws_provider_destroy and flb_upstream_destroy and returns directly, which
leaks gg_cfg, protocol, host, port_sds and endpoint_path because it bypasses the
cleanup label; change this path to jump to the existing error cleanup (use goto
error) so the common cleanup runs, and to avoid double-free of upstream (since
destroy_fn_iot will free implementation->client which owns upstream once
assigned) either set upstream = NULL before jumping to error or add a guard so
flb_upstream_destroy is only called when upstream is not owned by
implementation->client; update the branch around implementation->client =
generator->create() accordingly.
- Around line 597-604: The code currently ignores flb_sds_cat return values and
mixes flb_sds_t with plain strdup'd strings causing improper free; update the
block around tmp and credentials_endpoint to capture each flb_sds_cat return
into tmp (e.g., tmp = flb_sds_cat(tmp, ...)), check for NULL and goto error on
allocation failure, then ensure implementation->credentials_endpoint is stored
as the correct type: either convert the sds back to a plain flb_strdup'd C
string before assigning (so destroy_fn_iot can continue to call flb_free), or
consistently store an flb_sds_t and update destroy_fn_iot to call
flb_sds_destroy on implementation->credentials_endpoint; also replace any
flb_free usage on an flb_sds_t with flb_sds_destroy to avoid heap corruption.

In `@src/aws/flb_aws_credentials.c`:
- Around line 54-62: Update the block comment labeled "The standard credential
provider chain" so its numbered list matches the actual insertion order used in
the code: Environment variables first, IoT credentials endpoint second (the code
inserts IoT as the second provider), Shared credentials file (AWS Profile)
third, followed by EKS OIDC, EC2 IMDS, and ECS HTTP credentials endpoint; adjust
the numbering and descriptions in that comment to reflect this exact order so
maintainers won't be confused by the mismatch.
🧹 Nitpick comments (3)
src/aws/flb_aws_credentials_iot.c (3)

614-620: atoi returns 0 for non-numeric input — consider strtol for stricter validation.

atoi("0") is a valid port 0 which would be rejected, and atoi("abc") also returns 0. Using strtol with end-pointer checking would distinguish between "port is literally 0" and "not a number." This is a minor robustness concern since port 0 is invalid anyway.


854-863: JSON key matching with strncmp and len has a subtle issue for short keys like "Token".

strncmp(current_token, "Token", len) where len is the token length from jsmn. If the JSON key is "TokenExtra", len would be 10 and strncmp compares first 10 chars of "Token" (only 5 chars) — this is fine because strncmp stops at the shorter string's null terminator.

However, the reverse case is the concern: if a JSON key is "Tok" (len=3), strncmp("Tok", "Token", 3) returns 0, matching incorrectly. This is unlikely given the IoT credential response schema, but adding && len == 5 (or using the token field length) would be more robust, consistent with how AWS SDKs parse these responses.


734-740: Verbose debug logging of header details on every credential request.

These debug logs (lines 734-740) duplicate information already logged during provider creation (lines 685-687). Consider removing or gating behind a more verbose trace level to reduce log noise during periodic refreshes.

@SagiROosto SagiROosto force-pushed the add-aws-iot-credentials branch from 19a31f4 to b7eb2cc Compare February 16, 2026 14:36
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@src/aws/flb_aws_credentials_iot.c`:
- Around line 629-635: flb_utils_url_split_sds currently returns an
endpoint_path that is ignored, so when parsing credentials_endpoint any
user-supplied path is silently discarded before calling iot_credentials_request;
update the parsing/handling to detect a non-empty endpoint_path and either log a
warning (e.g. via AWS_CREDS_ERROR or AWS_CREDS_WARN) informing the user that
only host:port are used, or reject/normalize the URL by appending/merging
endpoint_path into the request path used by iot_credentials_request; locate the
call to flb_utils_url_split_sds and the variable endpoint_path and add the
warning/log or path-merge logic before constructing the request path for
iot_credentials_request so the behavior is explicit.
- Around line 879-973: The JSON key comparisons in the parsing loop (inside the
while over tokens in flb_aws_credentials_iot.c) use strncmp(current_token,
"<key>", len) which allows prefix matches; change each comparison to require the
token length equals the literal length and then compare that exact length (e.g.
check len == sizeof("accessKeyId")-1 && strncmp(current_token, "accessKeyId",
len) == 0), and apply this pattern to the comparisons for "credentials",
"accessKeyId"/"AccessKeyId", "secretAccessKey"/"SecretAccessKey",
"sessionToken"/"Token", and "expiration"/"Expiration" so only exact key matches
are accepted. Ensure you keep the same token advancement logic (i++ / t =
&tokens[i]) and error paths unchanged.
🧹 Nitpick comments (4)
src/aws/flb_aws_credentials_iot.c (4)

93-258: YAML parser is functional but fragile with depth-based state tracking.

The parser relies on depth counters and boolean flags to navigate the Greengrass config YAML tree. This works for the documented structure but will silently produce incorrect results if the YAML contains unexpected nesting (e.g., anchors, merge keys, or sequence nodes within the expected mappings). Consider adding a brief comment noting that only the specific Greengrass V2 config.yaml schema is supported.

Also, note that YAML_SEQUENCE_START_EVENT / YAML_SEQUENCE_END_EVENT are not handled — if the config contains unexpected sequences, depth tracking will drift. This is unlikely for Greengrass V2 config but worth a defensive check.


574-580: Minor: pthread_mutex_destroy not called on provider->lock when implementation allocation fails.

Line 572 initializes the mutex, but the failure path at lines 577-578 frees provider without destroying the mutex first. This is consistent with how standard_chain_create handles the same scenario, so it's an existing pattern rather than a new bug, but worth noting.

Proposed fix
     implementation = flb_calloc(1, sizeof(struct flb_aws_provider_iot));
     if (!implementation) {
         flb_errno();
+        pthread_mutex_destroy(&provider->lock);
         flb_free(provider);
         provider = NULL;
         goto cleanup;
     }

985-988: Consider redacting AccessKeyId in debug log.

While AccessKeyId is technically not a secret, logging it (even at debug level) makes it easier to correlate with leaked credentials. Other AWS SDKs typically truncate or mask it. Low priority given debug-level only.


475-475: Nit: endpoint_path should be declared as flb_sds_t for consistency.

It's allocated by flb_utils_url_split_sds as flb_sds_t and freed with flb_sds_destroy, but declared as char *. The other URL components (protocol, host, port_sds) are correctly typed.

Proposed fix
-    char *endpoint_path = NULL;
+    flb_sds_t endpoint_path = NULL;

@SagiROosto SagiROosto force-pushed the add-aws-iot-credentials branch from b7eb2cc to 0f68e86 Compare February 16, 2026 15:05
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@src/aws/flb_aws_credentials_iot.c`:
- Around line 653-658: The code uses atoi(port_sds) which does not detect
negatives/overflows; replace atoi with strtol and validate: call
strtol(port_sds, &endptr, 10), check errno for ERANGE, ensure endptr points to
end (no leftover chars), and confirm the converted long is within 1..65535
before assigning to port; on any failure call AWS_CREDS_ERROR("Invalid port in
IoT credentials endpoint: %s", port_sds) and goto error as before. Ensure you
refer to port_sds, port, AWS_CREDS_ERROR and remove the old atoi usage.
- Around line 566-580: The allocated provider's mutex (initialized by
pthread_mutex_init on provider->lock) is not destroyed on the failure path when
flb_calloc for implementation fails; update the failure branch inside the
function that allocates provider and implementation so that before freeing
provider (flb_free(provider)) you call pthread_mutex_destroy(&provider->lock),
then null the provider pointer and continue to the existing cleanup label;
ensure the change references the existing symbols provider, implementation,
pthread_mutex_init, pthread_mutex_destroy, flb_calloc, flb_free and preserves
current goto cleanup control flow.
🧹 Nitpick comments (2)
src/aws/flb_aws_credentials_iot.c (2)

1007-1009: Debug log exposes AccessKeyId.

While SecretAccessKey logging was correctly removed (per prior review), the AccessKeyId is still logged at debug level. Although less sensitive than the secret key, AccessKeyId is still a credential component. Consider masking it (e.g., showing only the last 4 characters) or removing it.

🔒 Example: mask the key ID
-    AWS_CREDS_DEBUG("Successfully parsed IoT credentials "
-                    "- AccessKeyId: %s, Expiration: %ld",
-                    creds->access_key_id, *expiration);
+    AWS_CREDS_DEBUG("Successfully parsed IoT credentials "
+                    "- AccessKeyId: ***%s, Expiration: %ld",
+                    creds->access_key_id + (flb_sds_len(creds->access_key_id) > 4
+                        ? flb_sds_len(creds->access_key_id) - 4 : 0),
+                    *expiration);

288-288: Make these seven vtable functions static.

get_credentials_fn_iot, refresh_fn_iot, init_fn_iot, sync_fn_iot, async_fn_iot, upstream_set_fn_iot, and destroy_fn_iot are only referenced through the iot_provider_vtable struct and have no external references in the codebase. Declaring them static limits symbol visibility and follows standard practice for module-internal functions.

♻️ Example for one function (apply to all seven)
-struct flb_aws_credentials *get_credentials_fn_iot(struct flb_aws_provider *provider)
+static struct flb_aws_credentials *get_credentials_fn_iot(struct flb_aws_provider *provider)

Also applies to: 355-355, 369-369, 387-387, 396-396, 405-405, 415-415

@SagiROosto SagiROosto force-pushed the add-aws-iot-credentials branch from 0f68e86 to 8a4a5d1 Compare February 18, 2026 10:01
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (2)
include/fluent-bit/flb_aws_credentials.h (1)

46-49: Macro names don't match their env var values — inconsistent with the rest of this header.

Every other IoT macro in this block follows the MACRO_NAME == "MACRO_NAME" convention, but:

  • AWS_IOT_GREENGRASS_V2_CONFIG"AWS_IOT_GREENGRASS_V2_CONFIG_PATH" (macro is missing the _PATH suffix)
  • AWS_GG_ROOT_CA_PATH"GG_ROOT_CA_PATH" (the actual Greengrass agent env var has no AWS_ prefix; the macro's AWS_ prefix implies a symmetry that doesn't exist)
♻️ Proposed rename (non-breaking — only renames the C macro identifiers, not the env var strings)
-/* Greengrass V2 Config File - fallback source for IoT configuration */
-#define AWS_IOT_GREENGRASS_V2_CONFIG   "AWS_IOT_GREENGRASS_V2_CONFIG_PATH"

-/* Greengrass V2 Component Environment Variable - fallback for CA cert */
-#define AWS_GG_ROOT_CA_PATH            "GG_ROOT_CA_PATH"
+/* Greengrass V2 Config File - fallback source for IoT configuration */
+#define AWS_IOT_GREENGRASS_V2_CONFIG_PATH  "AWS_IOT_GREENGRASS_V2_CONFIG_PATH"

+/* Greengrass Component Environment Variable (set by Greengrass agent) - fallback for CA cert */
+#define GG_ROOT_CA_PATH                    "GG_ROOT_CA_PATH"

Also update every usage in src/aws/flb_aws_credentials_iot.c: AWS_IOT_GREENGRASS_V2_CONFIGAWS_IOT_GREENGRASS_V2_CONFIG_PATH and AWS_GG_ROOT_CA_PATHGG_ROOT_CA_PATH.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/fluent-bit/flb_aws_credentials.h` around lines 46 - 49, The two macro
identifiers in flb_aws_credentials.h are inconsistent with their string values;
rename the C macros to match the existing env var strings and update uses:
change AWS_IOT_GREENGRASS_V2_CONFIG to AWS_IOT_GREENGRASS_V2_CONFIG_PATH (string
remains "AWS_IOT_GREENGRASS_V2_CONFIG_PATH") and change AWS_GG_ROOT_CA_PATH to
GG_ROOT_CA_PATH (string remains "GG_ROOT_CA_PATH"), then update all references
in src/aws/flb_aws_credentials_iot.c to use the new macro names (search/replace
occurrences of AWS_IOT_GREENGRASS_V2_CONFIG and AWS_GG_ROOT_CA_PATH).
src/aws/flb_aws_credentials_iot.c (1)

786-792: Leftover debug diagnostics expose implementation internals on every credential refresh.

The header-count and header-key/value logs (lines 787–792) are clearly development artifacts — the count is always 1 and the key/value are constants. They also emit the thing_name (device identifier) on every credentials fetch. These should be removed.

♻️ Proposed cleanup
     AWS_CREDS_DEBUG("Making IoT credentials request to: %s", uri);
-    AWS_CREDS_DEBUG("Client headers count: %d", implementation->client->static_headers_len);
-    if (implementation->client->static_headers_len > 0) {
-        AWS_CREDS_DEBUG("Client header: %s = %s", 
-                  implementation->client->static_headers[0].key,
-                  implementation->client->static_headers[0].val);
-    }
     c = implementation->client->client_vtable->request(...);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/aws/flb_aws_credentials_iot.c` around lines 786 - 792, Remove the
leftover debug diagnostics that leak implementation internals and the device
identifier: delete the AWS_CREDS_DEBUG calls that log
implementation->client->static_headers_len and the block that prints
implementation->client->static_headers[0].key/val (and any log that prints the
thing_name); instead keep (or add) a single non-sensitive debug message like
"Making IoT credentials request to: %s" with the uri only, so no header counts,
header keys/values or thing_name are emitted.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/aws/flb_aws_credentials_iot.c`:
- Around line 1016-1018: The debug log call AWS_CREDS_DEBUG that currently
prints creds->access_key_id should be changed to avoid emitting the raw Access
Key ID; instead compute and log a safe indicator (e.g., last 4 chars or a short
hash) and include that indicator in the message alongside expiration. Update the
AWS_CREDS_DEBUG invocation that mentions "AccessKeyId" so it no longer
interpolates creds->access_key_id directly but uses the sanitized indicator
variable (derived from creds->access_key_id) to preserve traceability without
revealing the full identifier.
- Around line 178-228: parse_greengrass_config currently overwrites existing
strings (e.g., config->cert_file, config->key_file, config->ca_cert_file,
config->thing_name, config->cred_endpoint, config->role_alias) with flb_strdup'd
values when duplicate YAML keys appear, leaking the previous allocations; before
assigning a new flb_strdup result, check if the target field is non-NULL and
call flb_free on it, then assign the strdup'ed value (repeat this guard for all
six fields referenced above), keeping the existing last_key handling unchanged.

---

Duplicate comments:
In `@src/aws/flb_aws_credentials.c`:
- Around line 54-62: The comment block "The standard credential provider chain"
in src/aws/flb_aws_credentials.c now accurately reflects the provider insertion
order in the code, so no code change is required; leave the comment as-is and,
when modifying provider registration in flb_aws_credentials.c in the future,
update this comment to match any changes to the insertion order.

---

Nitpick comments:
In `@include/fluent-bit/flb_aws_credentials.h`:
- Around line 46-49: The two macro identifiers in flb_aws_credentials.h are
inconsistent with their string values; rename the C macros to match the existing
env var strings and update uses: change AWS_IOT_GREENGRASS_V2_CONFIG to
AWS_IOT_GREENGRASS_V2_CONFIG_PATH (string remains
"AWS_IOT_GREENGRASS_V2_CONFIG_PATH") and change AWS_GG_ROOT_CA_PATH to
GG_ROOT_CA_PATH (string remains "GG_ROOT_CA_PATH"), then update all references
in src/aws/flb_aws_credentials_iot.c to use the new macro names (search/replace
occurrences of AWS_IOT_GREENGRASS_V2_CONFIG and AWS_GG_ROOT_CA_PATH).

In `@src/aws/flb_aws_credentials_iot.c`:
- Around line 786-792: Remove the leftover debug diagnostics that leak
implementation internals and the device identifier: delete the AWS_CREDS_DEBUG
calls that log implementation->client->static_headers_len and the block that
prints implementation->client->static_headers[0].key/val (and any log that
prints the thing_name); instead keep (or add) a single non-sensitive debug
message like "Making IoT credentials request to: %s" with the uri only, so no
header counts, header keys/values or thing_name are emitted.

@SagiROosto SagiROosto force-pushed the add-aws-iot-credentials branch from 8a4a5d1 to 3ae6284 Compare February 18, 2026 10:16
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@include/fluent-bit/flb_aws_credentials.h`:
- Line 49: The macro AWS_GG_ROOT_CA_PATH currently expands to "GG_ROOT_CA_PATH"
which mismatches the getenv(AWS_GG_ROOT_CA_PATH) usage in
flb_aws_credentials_iot.c; fix by changing the macro definition of
AWS_GG_ROOT_CA_PATH to "AWS_GG_ROOT_CA_PATH" so the macro name and environment
variable match (or alternatively rename the macro to GG_ROOT_CA_PATH and update
the getenv usage accordingly), ensuring the getenv(AWS_GG_ROOT_CA_PATH) call
continues to read the intended env var.

In `@src/aws/flb_aws_credentials_iot.c`:
- Around line 316-338: The code discards the return value of
iot_credentials_request in the refresh path, causing a misleading warning when
the refresh actually failed; update the block that calls iot_credentials_request
(inside the try_lock_provider branch) to capture its return (e.g.,
success/failure boolean or pointer), handle failure by logging an error and
ensuring implementation->creds remains NULL (and return NULL), and only fall
through to the existing "refresh already in progress" warning when the lock was
not obtained and another co-routine is refreshing; mirror the behavior used in
refresh_fn_iot and init_fn_iot (check the returned status from
iot_credentials_request, call unlock_provider(provider) in all paths, and avoid
printing the “already in progress” message when the local refresh has failed).
- Around line 626-638: The tmp SDS returned by flb_sds_create_size is
overwritten by flb_sds_cat on failure, leaking the original buffer; modify the
code around flb_sds_create_size / flb_sds_cat (symbols: tmp,
flb_sds_create_size, flb_sds_cat) to preserve the original pointer before
calling flb_sds_cat (e.g., save to orig_tmp), check the return value, and if
flb_sds_cat returns NULL free the original SDS via flb_sds_destroy (or the
appropriate SDS free) and then goto error; ensure both cat calls follow this
pattern so no allocated SDS is lost, and keep AWS_CREDS_ERROR/goto error
handling intact.
- Around line 694-706: The TLS debug argument passed to flb_tls_create should be
standardized to 0 instead of -1; update the call where implementation->tls is
created (the flb_tls_create invocation) to pass 0 for the debug parameter so it
matches documented debug levels and other usages (e.g., the pattern used in
flb_oauth2_jwt.c), then keep the existing null/ca/cert/key arguments and the
error check that logs "Failed to create TLS instance for IoT Provider"
unchanged.

---

Duplicate comments:
In `@src/aws/flb_aws_credentials_iot.c`:
- Around line 1034-1036: The debug log still interpolates the raw credential
identifier creds->access_key_id into AWS_CREDS_DEBUG; replace this by redacting
or masking the value (e.g., log "<redacted>" or only the last 4
characters/hashed value) so the full AccessKeyId is never emitted, and update
the AWS_CREDS_DEBUG call that references creds->access_key_id accordingly (keep
the rest of the message and the expiration field intact).

This update introduces the IoT provider creation function and adds necessary environment variable definitions for IoT credentials in the AWS module. Additionally, the CMake configuration is updated to include the new IoT credentials source file.

Signed-off-by: SagiROosto <sagi.rosenthal@oosto.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant