Skip to content

chore: add user name character warning #1708

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

kstribrnAmzn
Copy link
Member

@kstribrnAmzn kstribrnAmzn commented Apr 16, 2025

Issue #, if available:

Description of changes:
Adds a warning log when potentially
invalid characters are used in a username.

Why is this change necessary:

How was this change tested:

  • Updated or added new unit tests.
  • Updated or added new integration tests.
  • Updated or added new end-to-end tests.
  • If my code makes a remote network call, it was tested with a proxy.

Any additional information or context required to review the change:
This change aims to raise awareness around invalid user names which can cause component failures.
For Linux see this (best doc I could find).
For Windows see this.
Documentation Checklist:

  • Updated the README if applicable.

Compatibility Checklist:

  • I confirm that the change is backwards compatible.
  • Any modification or deletion of public interfaces does not impact other plugin components.
  • For external library version updates, I have reviewed its change logs and Nucleus does not consume
    any deprecated method or type.

Refer to Compatibility Guidelines for more information.

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

Adds a warning log when potentially
invalid characters are used in a username.
@kstribrnAmzn
Copy link
Member Author

To test this change I deployed the nucleus onto a windows instance and attempted to boot it up with the following command

java -Droot="C:\Users\Administrator\aws.greengrass.nucleus" "-Dlog.store=FILE" -jar .\aws.greengrass.nucleus\lib\Greengrass.jar --aws-region us-west-2 --thing-name windowsWarnThing --component-default-user ggc_group:ggc_user

Log message seen in the greengrass.log file

2025-05-01T15:54:54.800Z [INFO] (main) com.aws.greengrass.security.SecurityService: Register crypto key service provider. {keyType=file}
2025-05-01T15:54:54.800Z [INFO] (main) com.aws.greengrass.security.SecurityService: Register MQTT connection security provider. {keyType=file}
2025-05-01T15:54:54.803Z [WARN] (main) com.aws.greengrass.lifecyclemanager.KernelCommandLine: Component user may contain invalid characters. This can cause issues starting a component.. {}
2025-05-01T15:54:54.842Z [INFO] (main) com.aws.greengrass.lifecyclemanager.Kernel: Successfully setup Nucleus launch parameters. {}
2025-05-01T15:54:55.049Z [INFO] (main) com.aws.greengrass.lifecyclemanager.Kernel: Nucleus lifecycle has been initialized successfully. {}

@alter-mage
Copy link
Member

LGTM but how are you creating this pull request? Are you creating a fork of this repo and creating a PR out of that? That will not work as it will not be able to run the CI for unit and integ tests. You need clone this repo, create a new branch for your work and create a PR from that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants