Skip to content

Implement OutputLogFormatter to Enhance JSON Response Privacy #2243

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

gorets
Copy link

@gorets gorets commented Mar 16, 2024

This pull request introduces the OutputLogFormatter, a new feature aimed at bolstering the privacy of JSON responses within the application. The OutputLogFormatter empowers users to customize and manipulate the output data, offering capabilities to selectively remove or conceal sensitive information.

The implementation ensures simplicity and ease of use, allowing users to seamlessly integrate privacy modifications into their JSON responses.

Example of usage

function customOutputFormatter(input: string): string {
  // hide sign in the Bearer tokens
  const regex = new RegExp(/"Bearer\s+(.+?)\.(.+?)\.(.+?)(\\?)"/, 'gi');
  let result = input.replace(regex, '"Bearer $1.$2.******$4"');
  
  // hide Basic credentials
  const regex2 = new RegExp(/"Basic\s+(.+?)(\\?)"/, 'gi');
  result = result.replace(regex2, '"Basic ******$2"');

  // hide values for popular secret keys
  const regex3 = new RegExp(/"(password|secret|key|token)(\\?)"\s*:\s*(\\?)"(.+?)(\\?)"/, 'gi');
  result = result.replace(regex3, '"$1$2":$3"******$5"');

  return result;
}

const logger = new Logger({
  logLevel: 'DEBUG',
  outputFormatter: customOutputFormatter
});

Example result

image

Issue number: 728

Checklist

  • My changes meet the tenets criteria
  • I have performed a self-review of my own code
  • I have commented my code where necessary, particularly in areas that should be flagged with a TODO, or hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my change is effective and works
  • The PR title follows the conventional commit semantics

Breaking change checklist

Is it a breaking change?: NO

  • I have documented the migration process
  • I have added, implemented necessary warnings (if it can live side by side)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

@gorets gorets requested a review from a team March 16, 2024 20:15
@gorets gorets requested a review from a team as a code owner March 16, 2024 20:15
@boring-cyborg boring-cyborg bot added the logger This item relates to the Logger Utility label Mar 16, 2024
Copy link

boring-cyborg bot commented Mar 16, 2024

Thanks a lot for your first contribution! Please check out our contributing guidelines and don't hesitate to ask whatever you need.
In the meantime, check out the #typescript channel on our Powertools for AWS Lambda Discord: Invite link

@pull-request-size pull-request-size bot added the size/M PR between 30-99 LOC label Mar 16, 2024
Copy link

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@dreamorosi dreamorosi added the do-not-merge This item should not be merged label Mar 17, 2024
@dreamorosi
Copy link
Contributor

Hi @gorets, thank you for taking the time to work on and open this PR.

The same behavior you are proposing can be implemented using existing functionalities, and specifically the "bring your own formatter" feature, for this reason I am not inclined to merge this pull request.

The custom log formatter feature allows you to extend the base log formatter and provide your own implementation. With this method you get access to each log item before it's emitted and it's our preferred way of extending the logger.

Specifically, the feature you proposed, could be implemented like this:

import { LogFormatter, LogItem, Logger } from "@aws-lambda-powertools/logger"
import {
    UnformattedAttributes,
    LogAttributes,
} from "@aws-lambda-powertools/logger/types"

export class CustomLogFormatter extends LogFormatter {
    #hideValues(obj: any) {
        for (const key in obj) {
            if (typeof obj[key] === "object") {
                this.#hideValues(obj[key])
            } else {
                if (
                    ["password", "secret", "key", "token"].includes(
                        key.toLowerCase()
                    )
                ) {
                    obj[key] = "******"
                }
                // check for values that look like a Bearer token
                if (obj[key].includes("Bearer")) {
                    obj[key] = "Bearer ******"
                } else if (obj[key].includes("Basic")) {
                    obj[key] = "Basic ******"
                }
            }
        }
    }

    formatAttributes(
        attributes: UnformattedAttributes,
        additionalLogAttributes: LogAttributes
    ): LogItem {
        const { message, ...rest } = attributes
        // hide sensitive values from the message
        // all other base attributes are controlled by Powertools, so there's no need to check them
        this.#hideValues(message)
        // hide sensitive values from the additional attributes
        this.#hideValues(additionalLogAttributes)

        const logItem = new LogItem({
            attributes: {
                message,
                ...rest,
            },
        })
        logItem.addAttributes(additionalLogAttributes) // add any attributes not explicitly defined

        return logItem
    }
}

The custom log formatter would then be used like this:

import { Logger } from '@aws-lambda-powertools/logger'
import { CustomLogFormatter } from './customFormatter'

const logger = new Logger({ logFormatter: new CustomLogFormatter() })

export const handler = async () => {
    logger.info("Hello, world!", {
        token: "value",
        headers: { authorization: "Basic Ym9zY236Ym9zY28=" },
    })

    return {
        statusCode: 200,
        body: JSON.stringify("Hello, world!"),
    }
}

and would generate the following result:

{
    "message": "Hello, world!",
    "logLevel": "INFO",
    "timestamp": "2024-03-20T17:26:41.655Z",
    "serviceName": "service_undefined",
    "sampleRateValue": 0,
    "token": "******",
    "headers": {
        "authorization": "Basic ******"
    }
}

Using the log formatter method described above has the advantage of avoiding to stringify the log message only to apply regexes against it, only to later parse it back to an object. Both operations are redundant since the log item is later passed to the log formatter and then stringified again.

Using the implementation suggested would have a negative impact on performance since every log would be stringified twice and parsed before being emitted.

Finally, regarding the linked issue, I agree that your proposal could partially address the needs of the RFC, however the RFC itself hasn't been marked as complete and it will most likely be moving into a direction similar to the feature implemented here once it's picked up.


With that said, I want to remark once again that I personally appreciate your effort and the fact that you showed interest in contributing to the project. If you'd like to contribute further I'd suggest to start looking at existing issues marked as status/confirmed or good-first-issue, or help-wanted.

If instead you'd like to propose a new feature and have a chance to work on it, I'd recommend reading and following the process highlighted here which involves opening a feature request, followed by an optional RFC, and then finally by a PR once the feature and its design have been agreed with one of the maintainers.

@dreamorosi dreamorosi closed this Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge This item should not be merged logger This item relates to the Logger Utility size/M PR between 30-99 LOC
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants