[feat:extensions] add support to package web-bot-auth #62
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
archandatta
force-pushed
the
archand/add-support/web-bot-auth
branch
from
01fb400 to
6380ccd
Compare
rgarcia
left a comment
rgarcia
left a comment
There was a problem hiding this comment.
Good feature addition! The overall structure is clean and the UX is thoughtful with helpful next-steps messaging.
Main areas to address:
- JWK vs PEM messaging: Several places in help text, logs, and comments refer to "JWK" but PEM format is also supported. Would be good to make this consistent.
- Error handling: A few places where
os.Staterrors other thanIsNotExistare silently ignored, and some URL update failures are warnings instead of errors. - Stability: Consider pinning the GitHub download to a specific commit to avoid upstream breaking changes.
- Crypto code: The stdlib already has
crypto/x509.MarshalPKCS8PrivateKeyfor Ed25519 - no need to hand-roll ASN.1.
| extensionsUploadCmd.Flags().String("name", "", "Optional unique extension name") | ||
| extensionsPrepareWebBotAuthCmd.Flags().String("to", "./web-bot-auth", "Output directory for the prepared extension") | ||
| extensionsPrepareWebBotAuthCmd.Flags().String("url", "http://127.0.0.1:10001", "Base URL for update.xml and policy templates") | ||
| extensionsPrepareWebBotAuthCmd.Flags().String("key", "", "Path to custom Ed25519 JWK key file (defaults to RFC9421 test key)") |
There was a problem hiding this comment.
the help text for --key says "JWK key file" but the code accepts both JWK and PEM formats. consider updating to something like "Path to Ed25519 private key file (JWK or PEM format)"
| defaultLocalhostURL = "http://localhost:8000" | ||
| defaultDirMode = 0755 | ||
| defaultFileMode = 0644 | ||
| webBotAuthDownloadURL = "https://github.com/cloudflare/web-bot-auth/archive/refs/heads/main.zip" |
There was a problem hiding this comment.
consider pinning this to a specific commit instead of main so we're not caught off guard by upstream changes breaking the build
| type ExtensionsBuildWebBotAuthInput struct { | ||
| Output string | ||
| HostURL string | ||
| KeyPath string // Path to user's JWK file (optional, defaults to RFC9421 test key) |
There was a problem hiding this comment.
comment says "JWK file" but PEM is also supported
| if len(entries) > 0 { | ||
| return nil, fmt.Errorf("output directory must be empty: %s", outputDir) | ||
| } | ||
| } else { |
There was a problem hiding this comment.
this else branch triggers on any os.Stat error, not just os.IsNotExist. other errors (e.g. permission denied) would silently proceed to MkdirAll
| var jwkData string | ||
| var usingDefaultKey bool | ||
| if in.KeyPath != "" { | ||
| pterm.Info.Printf("Loading custom JWK from %s...\n", in.KeyPath) |
There was a problem hiding this comment.
log message says "Loading custom JWK" but the file could be PEM
| // Copy policy directory (contains Chrome enterprise policy configuration) | ||
| policySrc := filepath.Join(browserExtDir, "policy") | ||
| policyDst := filepath.Join(outputDir, "policy") | ||
| if _, err := os.Stat(policySrc); err == nil { |
There was a problem hiding this comment.
non-existence errors are handled but other stat errors are silently ignored
| if usingDefaultKey { | ||
| rows = append(rows, []string{"Signing Key", "RFC9421 test key (Cloudflare test site)"}) | ||
| } else { | ||
| rows = append(rows, []string{"Signing Key", "Custom JWK"}) |
There was a problem hiding this comment.
when using a PEM file, this still says "Custom JWK" - should probably say "Custom key" or detect the format
|
|
||
| req, err := http.NewRequestWithContext(ctx, http.MethodGet, webBotAuthDownloadURL, nil) | ||
| if err != nil { | ||
| t.Fatalf("Failed to create request: %v", err) |
There was a problem hiding this comment.
nit: we generally prefer testify's require and assert over t.Fatalf
|
|
||
| // JWKToPEM converts an Ed25519 JWK to PEM format (PKCS#8) | ||
| // If the input is already in PEM format, it validates and returns it as-is | ||
| func JWKToPEM(jwkJSON string) ([]byte, error) { |
There was a problem hiding this comment.
this crypto code would benefit from unit tests, especially to verify the PKCS#8 output matches what the stdlib produces
| } | ||
|
|
||
| // MarshalPKCS8PrivateKey creates a PKCS#8 structure for Ed25519 private key | ||
| func MarshalPKCS8PrivateKey(key ed25519.PrivateKey) ([]byte, error) { |
There was a problem hiding this comment.
consider using crypto/x509.MarshalPKCS8PrivateKey instead of hand-rolling ASN.1 - the stdlib supports Ed25519 and would be less error-prone
Note
Introduces end-to-end tooling to prepare the Cloudflare
web-bot-authextension for Kernel use.extensions build-web-bot-authcommand with flags:--to,--url,--key,--upload,--namenpm install/build, bundle, and copy artifacts (.crx,update.xml, built files, policy directory, private_key.pem with.gitignore)extensions/<name>/update.xml) instead of Chrome ID in generated URLspkg/extensions/webbotauth.go(core logic) and tests for download/extractpkg/util: file ops (CopyFile,CopyDir,ModifyFile) and crypto (JWKToPEM, PKCS#8 marshal)cmd/extensions.goalongside existing list/download/upload commandsWritten by Cursor Bugbot for commit 73a9f65. This will update automatically on new commits. Configure here.