Skip to content

Use typed context keys instead of string literals#4814

Open
tmchow wants to merge 2 commits intoarmadaproject:masterfrom
tmchow:fix/typed-context-keys
Open

Use typed context keys instead of string literals#4814
tmchow wants to merge 2 commits intoarmadaproject:masterfrom
tmchow:fix/typed-context-keys

Conversation

@tmchow
Copy link
Copy Markdown

@tmchow tmchow commented Apr 4, 2026

Replaces bare string context keys ("principal", "user", "requestId") with typed constants from a new ctxkeys package. This prevents silent value collisions if another package uses the same string as a context key.

The ctxkeys package is a leaf dependency with zero imports, so it can be used by auth, requestid, armadacontext, and grpc without introducing circular dependencies.

Changes:

  • New internal/common/ctxkeys/ctxkeys.go with ContextKey type and three constants
  • auth/common.go: principalKey now uses ctxkeys.PrincipalKey, "user" literal replaced with ctxkeys.UserKey
  • requestid/interceptors.go: "requestId" literal replaced with ctxkeys.RequestIDKey
  • armadacontext/armada_context.go: readers updated to use typed keys
  • grpc/grpc.go: readers updated to use typed keys

Closes #4780

This PR was authored by a human with AI coding assistance.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 4, 2026

Greptile Summary

This PR refactors bare string context keys ("principal", "user", "requestId") into typed constants defined in a new leaf package internal/common/ctxkeys. The contextKey type is correctly unexported, meaning external packages cannot construct colliding keys, while the constants themselves remain accessible. All write sites (auth, requestid) and read sites (armadacontext, grpc) have been updated consistently.

Confidence Score: 5/5

Safe to merge — purely a refactor with no logic changes, all write/read sites updated consistently.

All five files make symmetric, mechanical substitutions of string literals with typed constants. The unexported contextKey type correctly prevents external collision. No behavioral differences introduced.

No files require special attention.

Vulnerabilities

No security concerns identified. The unexported key type prevents external packages from forging context keys, which is a minor security improvement over the previous string-literal approach.

Important Files Changed

Filename Overview
internal/common/ctxkeys/ctxkeys.go New leaf package with unexported contextKey type and three exported constants — correct idiomatic Go pattern for collision-safe context keys.
internal/common/auth/common.go Local principalKey const now aliases ctxkeys.PrincipalKey; "user" write-site replaced with ctxkeys.UserKey. Write/read symmetry preserved — GetPrincipal/WithPrincipal both use principalKey.
internal/common/requestid/interceptors.go "requestId" write-site in AddToIncomingContext replaced with ctxkeys.RequestIDKey; consistent with read sites in armadacontext and grpc.
internal/common/armadacontext/armada_context.go Read sites in FromGrpcCtx updated to use ctxkeys.UserKey and ctxkeys.RequestIDKey — no logic change.
internal/common/grpc/grpc.go Read sites in InterceptorLogger updated to use typed keys — no logic change.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    subgraph ctxkeys ["internal/common/ctxkeys"]
        PK["PrincipalKey (contextKey)"]
        UK["UserKey (contextKey)"]
        RK["RequestIDKey (contextKey)"]
    end

    subgraph writers ["Context Writers"]
        AUTH["auth/common.go\nCreateGrpcMiddlewareAuthFunction"]
        REQID["requestid/interceptors.go\nAddToIncomingContext"]
    end

    subgraph readers ["Context Readers"]
        ARMADA["armadacontext/armada_context.go\nFromGrpcCtx"]
        GRPC["grpc/grpc.go\nInterceptorLogger"]
        AUTHREAD["auth/common.go\nGetPrincipal / WithPrincipal"]
    end

    PK -->|principalKey alias| AUTHREAD
    UK --> AUTH
    UK --> ARMADA
    UK --> GRPC
    RK --> REQID
    RK --> ARMADA
    RK --> GRPC
Loading

Reviews (5): Last reviewed commit: "fix(ctxkeys): unexport ContextKey type f..." | Re-trigger Greptile

@tmchow tmchow force-pushed the fix/typed-context-keys branch from 5ee6144 to 7e41304 Compare April 6, 2026 18:29
Introduce a ctxkeys package with a custom ContextKey type to prevent
context value collisions from bare string keys. Replace all plain
string context keys ("principal", "user", "requestId") with typed
constants from the new package.

The ctxkeys package is a leaf dependency with no imports, avoiding
circular dependencies between auth, requestid, and armadacontext.

Closes armadaproject#4780

Signed-off-by: Trevin Chow <trevin@trevinchow.com>
@tmchow tmchow force-pushed the fix/typed-context-keys branch 2 times, most recently from c67bc17 to ba87952 Compare April 8, 2026 01:09
Make the ContextKey type unexported (contextKey) so no external package
can construct a colliding key. The exported constants remain accessible
across internal packages while the type itself is hidden.

This contribution was developed with AI assistance (Claude Code).

Signed-off-by: Trevin Chow <trevin@trevinchow.com>
@tmchow tmchow force-pushed the fix/typed-context-keys branch from ba87952 to d644b5f Compare April 9, 2026 21:05
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.

Use typed context keys instead of string literals

1 participant