Skip to content

feat: zeroid cli, first commit#35

Open
rsharath wants to merge 7 commits intomainfrom
cli
Open

feat: zeroid cli, first commit#35
rsharath wants to merge 7 commits intomainfrom
cli

Conversation

@rsharath
Copy link
Copy Markdown
Contributor

Summary

CLI for zeroid

Type of change

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

Testing

  • Manually tested
  • Unit tests added/updated

@rsharath rsharath self-assigned this Mar 30, 2026
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new TypeScript-based CLI tool named zid for the ZeroID service, enabling users to manage agent identities, issue and verify tokens, and ingest CAE signals. The implementation includes a comprehensive test suite, documentation, and integration into the project's root Makefile. The review feedback focuses on refining the development workflow in the Makefile, eliminating redundant configuration in gitignore files, and improving the internal logic of the CLI. Key suggestions include simplifying Base64URL decoding by leveraging Node.js 18+ built-ins, consolidating duplicated time-formatting logic, and optimizing the handling of JWT claims for better performance and output consistency.

@rsharath
Copy link
Copy Markdown
Contributor Author

/gemini review

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new TypeScript-based CLI (zid) for the ZeroID service, providing commands for agent registration, token management (issuing, decoding, verifying, and revoking), and signal ingestion. It includes a comprehensive test suite and documentation comparing ZeroID with Microsoft Entra Agent Identity. Feedback focuses on addressing a local file dependency in package.json that would hinder distribution, improving security by restricting file permissions on configuration files containing API keys, and fixing a logic inconsistency in the JWT decoding command that causes certain claims to be hidden from the output.

@rsharath
Copy link
Copy Markdown
Contributor Author

/gemini review

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces the ZeroID CLI (zid), a comprehensive tool for managing agent identities, tokens, and signals. It includes commands for agent registration, JWT operations (issuance, verification, decoding, and revocation), and local profile management, supported by a robust test suite and documentation. The review feedback identifies a critical circular dependency in the init command that prevents first-time setup, as well as a discrepancy where the required --owner flag is missing from the documentation. Other recommendations focus on improving the CLI's robustness, such as handling interactive stdin to prevent hanging, securing sensitive file permissions for .env.zeroid, and refining expiration and date-parsing logic.

.option("--json", "Output raw JSON")
.action(async (opts) => {
try {
const client = makeClient(opts.profile as string | undefined);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

critical

There is a circular dependency in the bootstrap flow. zid init calls makeClient, which eventually calls requireProfile. If no profile or environment variables are set, requireProfile throws an error suggesting the user run zid init. This prevents a first-time user from ever successfully running the initialization command unless they manually set environment variables first. Consider allowing init to proceed without a pre-existing profile or prompting for the initial API key.

.command("init")
.description("Register a new agent and write credentials to .env.zeroid")
.requiredOption("--name <name>", "Human-readable agent name")
.requiredOption("--owner <owner_id>", "User ID of the agent owner")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The --owner option is marked as required in the code, but it is missing from the documentation and the quick start examples in README.md. This will cause the example commands provided to users to fail immediately. Either make this option optional (if it can be inferred) or update the documentation to include it.

Comment on lines +43 to +50
if (!token) {
// Read from stdin if no argument given.
const chunks: Buffer[] = [];
for await (const chunk of process.stdin) {
chunks.push(chunk as Buffer);
}
token = Buffer.concat(chunks).toString("utf8").trim();
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Reading from process.stdin when no JWT argument is provided will cause the CLI to hang indefinitely if the user runs the command in an interactive terminal without piping input. It is best practice to check if process.stdin.isTTY and throw a helpful error message if no input is provided via pipe.

Suggested change
if (!token) {
// Read from stdin if no argument given.
const chunks: Buffer[] = [];
for await (const chunk of process.stdin) {
chunks.push(chunk as Buffer);
}
token = Buffer.concat(chunks).toString("utf8").trim();
}
if (!token) {
if (process.stdin.isTTY) {
throw new Error("No JWT provided. Pass it as an argument or pipe it to stdin.");
}
// Read from stdin if no argument given.
const chunks: Buffer[] = [];
for await (const chunk of process.stdin) {
chunks.push(chunk as Buffer);
}
token = Buffer.concat(chunks).toString("utf8").trim();
}

if (nbf !== undefined) print("nbf", _formatTime(nbf));
if (iat !== undefined) print("iat", _formatTime(iat));
if (exp !== undefined) {
const expired = exp < Date.now() / 1000;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

A token should be considered expired if the current time is greater than or equal to the expiration time (exp). The current check only marks it as expired if the time is strictly greater.

Suggested change
const expired = exp < Date.now() / 1000;
const expired = exp <= Date.now() / 1000;


/** Write api_key to .env.zeroid in the current working directory. */
export function writeEnvFile(apiKey: string): void {
writeFileSync(join(process.cwd(), ".env.zeroid"), `ZID_API_KEY=${apiKey}\n`, "utf8");
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

security-medium medium

The .env.zeroid file contains a sensitive API key and should be created with restricted file permissions (e.g., 0o600) to prevent other users on the system from reading it. This is consistent with how the main config file is handled in _write.

Suggested change
writeFileSync(join(process.cwd(), ".env.zeroid"), `ZID_API_KEY=${apiKey}\n`, "utf8");
writeFileSync(join(process.cwd(), ".env.zeroid"), "ZID_API_KEY=" + apiKey + "\n", { encoding: "utf8", mode: 0o600 });

Comment on lines +39 to +40
export function relativeTime(iso: string): string {
const delta = Date.now() - new Date(iso).getTime();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The relativeTime function does not handle invalid date strings. If iso is not a valid date, new Date(iso).getTime() returns NaN, leading to confusing output like NaNd ago. Consider adding a check for valid dates.

Suggested change
export function relativeTime(iso: string): string {
const delta = Date.now() - new Date(iso).getTime();
export function relativeTime(iso: string): string {
const date = new Date(iso);
if (Number.isNaN(date.getTime())) return iso;
const delta = Date.now() - date.getTime();

Comment on lines +62 to +72
| Flag | Description | Default |
|---|---|---|
| `--name <name>` | Human-readable agent name | required |
| `--id <external_id>` | External ID (your own identifier) | same as `--name` |
| `--type <type>` | `agent` \| `application` \| `mcp_server` \| `service` | `agent` |
| `--sub-type <sub_type>` | `orchestrator` \| `tool_agent` \| `code_agent` \| `autonomous` \| ... | — |
| `--framework <name>` | Framework name, e.g. `langchain`, `mcp` | — |
| `--description <text>` | Short description | — |
| `--save-profile <name>` | Profile name to save credentials under | `default` |
| `--profile <name>` | Profile to use for the parent account/project | active profile |
| `--json` | Output raw JSON | — |
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The documentation for zid init is missing the --owner flag, which is marked as a required option in the implementation. Note that tenant-specific identifiers should not be mandatory for core functionality. If this flag is a tenant identifier, it should be documented as optional.

References
  1. Tenant-specific identifiers should not be mandatory for core functionality.

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.

2 participants