fix: account-ids handling, mask proxy as secret in logs (#1943)

* fix: enforce allowed-account-ids when the list contains empty entries

An empty first element previously short-circuited the allowed account
check. Empty entries are now filtered out and validation applies
whenever any non-empty entry exists.

* fix: enforce allowed-account-ids on the use-existing-credentials path

The early return for valid pre-existing credentials skipped the
allowed-account-ids check, now included.

* fix: reject newlines in names and values when writing profile files

If the profile file writing was enabled, we emitted newlines into the
file verbatim, permitting injecting arbitrary profiles into the file.
Writing now fails instead.

* fix: honor configured STS endpoint for "ambient" credentials

Ambient credential resolution built a bare STS client, so a web-identity
token found by the SDK default chain (e.g. AWS_WEB_IDENTITY_TOKEN_FILE on
a self-hosted runner) was exchanged with public STS instead of any
operator-configured sts-endpoint. Resolution now passes the configured
region, endpoint, and proxy handler to the default provider chain.

* fix: mask proxy URL credentials in job logs

Basic-auth userinfo in the http-proxy input or HTTP(S)_PROXY environment
variables was never registered as a secret, so error messages carrying
the proxy URL printed the credentials unmasked in the job log.

* fix: omit account IDs from the allowed-account-ids failure message

The mismatch error is thrown before exportAccountId registers the
account-id mask, so setFailed wrote the raw account ID (and the
configured allow-list) into a public annotation. (C4)

* chore: remove outdated examples

All of the examples were out of date and we do not have a mechanism for
keeping them up to date. Removed the examples.

src/CredentialsClient.ts

@@ -1,9 +1,10 @@
 import { info } from '@actions/core';
 import { STSClient } from '@aws-sdk/client-sts';
+import { defaultProvider } from '@aws-sdk/credential-provider-node';
 import type { AwsCredentialIdentity } from '@aws-sdk/types';
 import { NodeHttpHandler } from '@smithy/node-http-handler';
 import { ProxyAgent } from 'proxy-agent';
-import { buildCustomUserAgent, errorMessage, getCallerIdentity } from './helpers';
+import { buildCustomUserAgent, errorMessage, getCallerIdentity, maskProxyCredentials } from './helpers';
 import { ProxyResolver } from './ProxyResolver';
 
 if (!process.env.AWS_EXECUTION_ENV) {
@@ -31,6 +32,7 @@ export class CredentialsClient {
     }
     if (props.proxyServer) {
       info('Configuring proxy handler for STS client');
+      maskProxyCredentials(props.proxyServer);
       const proxyOptions: { httpProxy: string; httpsProxy: string; noProxy?: string } = {
         httpProxy: props.proxyServer,
         httpsProxy: props.proxyServer,
@@ -105,9 +107,15 @@ export class CredentialsClient {
   }
 
   private async loadCredentials() {
-    const config = {} as { requestHandler?: NodeHttpHandler };
-    if (this.requestHandler !== undefined) config.requestHandler = this.requestHandler;
-    const client = new STSClient(config);
-    return client.config.credentials();
+    // Previously we constructed a new client, but that picks up the default provider chain including the endpoint.
+    // Explicitly calling the default provider chain allows us to pass in the endpoint and region as well as the
+    // proxy config.
+    return defaultProvider({
+      clientConfig: {
+        ...(this.region !== undefined && { region: this.region }),
+        ...(this.stsEndpoint !== undefined && { endpoint: this.stsEndpoint }),
+        ...(this.requestHandler !== undefined && { requestHandler: this.requestHandler }),
+      },
+    })();
   }
 }

src/helpers.ts

@@ -5,7 +5,6 @@ import type { Credentials, STSClient } from '@aws-sdk/client-sts';
 import { GetCallerIdentityCommand } from '@aws-sdk/client-sts';
 import type { AwsCredentialIdentity } from '@aws-sdk/types';
 import type { UserAgent } from '@smithy/types';
-import type { CredentialsClient } from './CredentialsClient';
 
 const MAX_TAG_VALUE_LENGTH = 256;
 const SANITIZATION_CHARACTER = '_';
@@ -167,15 +166,13 @@ export function exportAccountId(identity: { Account: string; Arn: string }, mask
 // Validates that the account of the already-resolved caller identity is in the allow-list provided via the
 // `allowed-account-ids` input.
 export function validateAccountId(expectedAccountIds: string[] | undefined, account: string | undefined): void {
-  if (!expectedAccountIds || expectedAccountIds.length === 0 || expectedAccountIds[0] === '') {
+  const allowedAccountIds = expectedAccountIds?.filter((id) => id !== '') ?? [];
+  if (allowedAccountIds.length === 0) {
     return;
   }
-  if (!account || !expectedAccountIds.includes(account)) {
-    throw new Error(
-      `The account ID of the provided credentials (${
-        account ?? 'unknown'
-      }) does not match any of the expected account IDs: ${expectedAccountIds.join(', ')}`,
-    );
+  if (!account || !allowedAccountIds.includes(account)) {
+    // Account IDs are deliberately omitted: this error reaches the job log before any mask exists.
+    throw new Error('The account ID of the provided credentials does not match any of the allowed account IDs');
   }
 }
 
@@ -193,6 +190,29 @@ export function toCredentialIdentity(creds?: Partial<Credentials>): AwsCredentia
   };
 }
 
+// Registers any userinfo embedded in a proxy URL as secrets so it is masked in job logs.
+// First the literal proxy string, then any username/password components if parseable.
+// If the username/password is percent-encoded, the decoded form is also masked.
+export function maskProxyCredentials(proxyServer: string): void {
+  core.setSecret(proxyServer);
+  let url: URL;
+  try {
+    url = new URL(proxyServer);
+  } catch (_) {
+    return;
+  }
+  for (const part of [url.username, url.password]) {
+    if (!part) continue;
+    core.setSecret(part);
+    try {
+      const decoded = decodeURIComponent(part);
+      if (decoded !== part) core.setSecret(decoded);
+    } catch (_) {
+      // malformed percent-encoding; the raw form is already masked
+    }
+  }
+}
+
 // Tags have a more restrictive set of acceptable characters than GitHub environment variables can.
 // This replaces anything not conforming to the tag restrictions by inverting the regular expression.
 // See the AWS documentation for constraint specifics https://docs.aws.amazon.com/STS/latest/APIReference/API_Tag.html.
@@ -281,19 +301,6 @@ export function isDefined<T>(i: T | undefined | null): i is T {
 }
 /* c8 ignore stop */
 
-export async function areCredentialsValid(credentialsClient: CredentialsClient) {
-  const client = credentialsClient.stsClient;
-  try {
-    const identity = await client.send(new GetCallerIdentityCommand({}));
-    if (identity.Account) {
-      return true;
-    }
-    return false;
-  } catch (_) {
-    return false;
-  }
-}
-
 /**
  * Like core.getBooleanInput, but respects the required option.
  *

src/index.ts

@@ -3,12 +3,12 @@ import type { AssumeRoleCommandOutput } from '@aws-sdk/client-sts';
 import { assumeRole } from './assumeRole';
 import { CredentialsClient } from './CredentialsClient';
 import {
-  areCredentialsValid,
   errorMessage,
   exportAccountId,
   exportCredentials,
   exportRegion,
   getBooleanInput,
+  getCallerIdentity,
   retryAndBackoff,
   toCredentialIdentity,
   translateEnvVariables,
@@ -53,8 +53,8 @@ export async function run() {
     });
     const roleChaining = getBooleanInput('role-chaining', { required: false });
     const outputCredentials = getBooleanInput('output-credentials', { required: false });
-    // Default to always outputting environment credentials unless profile is specified. If profile is specified, default
-    // to no environment credentials (but still output them if the user specifically requests it).
+    // Default to always outputting environment credentials unless profile is specified. If profile is specified,
+    // default to no environment credentials (but still output them if the user specifically requests it).
     const outputEnvCredentials = getBooleanInput('output-env-credentials', { required: false, default: !awsProfile });
     const unsetCurrentCredentials = getBooleanInput('unset-current-credentials', { required: false });
     let disableRetry = getBooleanInput('disable-retry', { required: false });
@@ -165,8 +165,16 @@ export async function run() {
 
     //if the user wants to attempt to use existing credentials, check if we have some already
     if (useExistingCredentials) {
-      const validCredentials = await areCredentialsValid(credentialsClient);
-      if (validCredentials) {
+      const identity = await (async () => {
+        try {
+          return await getCallerIdentity(credentialsClient.stsClient);
+        } catch {
+          return null;
+        }
+      })();
+      if (identity) {
+        // The allowed-account-ids guardrail applies to reused credentials too.
+        validateAccountId(expectedAccountIds, identity.Account);
         core.notice('Pre-existing credentials are valid. No need to generate new ones.');
         if (timeoutId) clearTimeout(timeoutId);
         return;

src/profileManager.ts

@@ -53,8 +53,15 @@ export function parseIni(iniData: string): Record<string, Record<string, string>
 export function stringifyIni(data: Record<string, Record<string, string>>): string {
   const sections: string[] = [];
   for (const [sectionName, sectionData] of Object.entries(data)) {
+    if (/[\r\n]/.test(sectionName)) {
+      throw new Error('INI section names must not contain newline characters');
+    }
     const lines: string[] = [`[${sectionName}]`];
     for (const [key, value] of Object.entries(sectionData)) {
+      // A newline in a key or value would inject arbitrary INI lines (e.g. credential_process).
+      if (/[\r\n]/.test(key) || /[\r\n]/.test(value)) {
+        throw new Error('INI keys and values must not contain newline characters');
+      }
       lines.push(`${key} = ${value}`);
     }
     sections.push(lines.join('\n'));