From 18f0987ad84515a3b63b08c4e37de139c31185c1 Mon Sep 17 00:00:00 2001 From: Johannes Bechberger Date: Wed, 9 Sep 2026 14:25:39 +0200 Subject: [PATCH 01/39] feat(heap-dump): add --redact, --redact-complete, --compress flags - Embeds hprof-redact binaries (linux/amd64, linux/arm64, darwin/arm64, windows/amd64) via //go:embed; extracted to ~/.cache/cf-java-plugin/ on first use (SHA8-keyed, reused on subsequent runs). - --compress: passes gz=1 to jmap (JDK 17+) to compress the dump on the remote container before transfer, saving bandwidth. jvmmon path falls back to post-creation gzip on the container. Output is .hprof.gz. - --redact: pipes the downloaded dump through hprof-redact (lean mode: zeros primitive arrays only). Output is -redacted.hprof or -redacted.hprof.gz when --compress is also set. - --redact-complete: like --redact but zeros all primitive values (instance scalar fields + CLASS_DUMP statics) via two-pass redaction. - --compress and --redact can be combined: compress on remote to reduce transfer, redact locally (hprof-redact reads .hprof.gz natively). - Adds FindHeapDumpGzFile to utils for locating *.hprof.gz on the container (jvmmon+compress path). - Extracts osWindows and cmdHeapDump constants to satisfy goconst. --- Makefile | 51 ++++++++++++++++-- cf_cli_java_plugin.go | 109 +++++++++++++++++++++++++++++-------- jstall.go | 4 +- redact.go | 121 ++++++++++++++++++++++++++++++++++++++++++ utils/cfutils.go | 5 ++ 5 files changed, 263 insertions(+), 27 deletions(-) create mode 100644 redact.go diff --git a/Makefile b/Makefile index 9fd9e4b..56ef2c2 100644 --- a/Makefile +++ b/Makefile @@ -25,7 +25,7 @@ update-jstall: rm -f $(JSTALL_JAR) $(MAKE) download-jstall -.PHONY: build compile compile-all update-jstall download-jstall install remove clean vclean +.PHONY: build compile compile-all update-jstall download-jstall download-hprof-redact update-hprof-redact install remove clean vclean # When JSTALL_DEV=1, always re-download the jar (skip file existence check) ifdef JSTALL_DEV @@ -34,13 +34,56 @@ else JSTALL_DEP = $(JSTALL_JAR) endif -compile: $(JSTALL_DEP) +# ── hprof-redact embedded binaries ─────────────────────────────────────────── +# Downloaded at compile time from hprof-analyzer GitHub releases. +# Uses musl-static Linux builds so the binary runs in CF containers without +# glibc version constraints. +HPROF_REDACT_BASE = https://github.com/parttimenerd/hprof-analyzer/releases/latest/download + +dist/hprof-redact-linux-amd64: + mkdir -p dist + curl -sL $(HPROF_REDACT_BASE)/hprof-analyzer-x86_64-unknown-linux-musl.tar.gz \ + | tar -xz --strip-components=1 -C dist hprof-analyzer-x86_64-unknown-linux-musl/hprof-redact + mv dist/hprof-redact $@ + +dist/hprof-redact-linux-arm64: + mkdir -p dist + curl -sL $(HPROF_REDACT_BASE)/hprof-analyzer-aarch64-unknown-linux-musl.tar.gz \ + | tar -xz --strip-components=1 -C dist hprof-analyzer-aarch64-unknown-linux-musl/hprof-redact + mv dist/hprof-redact $@ + +dist/hprof-redact-darwin-arm64: + mkdir -p dist + curl -sL $(HPROF_REDACT_BASE)/hprof-analyzer-aarch64-apple-darwin.tar.gz \ + | tar -xz --strip-components=1 -C dist hprof-analyzer-aarch64-apple-darwin/hprof-redact + mv dist/hprof-redact $@ + +dist/hprof-redact-windows-amd64.exe: + mkdir -p dist + $(eval WINTMP := $(shell mktemp -d)) + curl -sL $(HPROF_REDACT_BASE)/hprof-analyzer-x86_64-pc-windows-msvc.zip -o $(WINTMP)/win.zip + cd $(WINTMP) && unzip -o win.zip hprof-analyzer-x86_64-pc-windows-msvc/hprof-redact.exe + cp $(WINTMP)/hprof-analyzer-x86_64-pc-windows-msvc/hprof-redact.exe $@ + rm -rf $(WINTMP) + +HPROF_REDACT_BINS = \ + dist/hprof-redact-linux-amd64 \ + dist/hprof-redact-linux-arm64 \ + dist/hprof-redact-darwin-arm64 \ + dist/hprof-redact-windows-amd64.exe + +download-hprof-redact: $(HPROF_REDACT_BINS) + +update-hprof-redact: + rm -f $(HPROF_REDACT_BINS) + $(MAKE) download-hprof-redact + +compile: $(JSTALL_DEP) $(HPROF_REDACT_BINS) go build -o build/cf-cli-java-plugin . -compile-all: $(JSTALL_DEP) +compile-all: $(JSTALL_DEP) $(HPROF_REDACT_BINS) GOOS=linux GOARCH=amd64 go build -o build/cf-cli-java-plugin-linux64 . GOOS=linux GOARCH=arm64 go build -o build/cf-cli-java-plugin-linux-arm64 . - GOOS=darwin GOARCH=amd64 go build -o build/cf-cli-java-plugin-osx . GOOS=darwin GOARCH=arm64 go build -o build/cf-cli-java-plugin-osx-arm64 . GOOS=windows GOARCH=amd64 go build -o build/cf-cli-java-plugin-win64.exe . diff --git a/cf_cli_java_plugin.go b/cf_cli_java_plugin.go index eaab765..02b0118 100644 --- a/cf_cli_java_plugin.go +++ b/cf_cli_java_plugin.go @@ -31,19 +31,24 @@ var _ plugin.Plugin = (*JavaPlugin)(nil) // String constants extracted to satisfy goconst linter. const ( - cmdSSH = "ssh" - cmdJava = "java" - flagKeep = "keep" - flagNoDownload = "no-download" - flagContainerDir = "container-dir" - flagLocalDir = "local-dir" - typeBool = "bool" - typeString = "string" - toolJcmd = "jcmd" - toolAsprof = "asprof" - extJFR = ".jfr" - labelJFR = "JFR recording" - partJFR = "jfr" + cmdSSH = "ssh" + cmdJava = "java" + flagKeep = "keep" + flagNoDownload = "no-download" + flagContainerDir = "container-dir" + flagLocalDir = "local-dir" + flagRedact = "redact" + flagRedactComplete = "redact-complete" + flagCompress = "compress" + osWindows = "windows" + cmdHeapDump = "heap-dump" + typeBool = "bool" + typeString = "string" + toolJcmd = "jcmd" + toolAsprof = "asprof" + extJFR = ".jfr" + labelJFR = "JFR recording" + partJFR = "jfr" ) // JavaPlugin is a CF CLI plugin that supports taking heap and thread dumps on demand @@ -207,6 +212,9 @@ type Options struct { ContainerDir string LocalDir string Args string + Redact bool + RedactComplete bool + Compress bool } // FlagDefinition holds metadata for a command-line flag @@ -285,6 +293,21 @@ var flagDefinitions = []FlagDefinition{ Description: "Miscellaneous arguments to pass to the command (if supported) in the container, be aware to end it with a space if it is a simple option. For commands that create arbitrary files (jcmd, asprof), the environment variables @FSPATH, @ARGS, @APP_NAME, @FILE_NAME, and @STATIC_FILE_NAME are available in --args to reference the working directory path, arguments, application name, and generated file name respectively.", Type: typeString, }, + { + Name: flagRedact, + Usage: "redact heap dump (lean mode: zero primitive arrays only) before saving locally", + Type: typeBool, + }, + { + Name: flagRedactComplete, + Usage: "redact heap dump (complete mode: zero all primitive values) before saving locally", + Type: typeBool, + }, + { + Name: flagCompress, + Usage: "compress heap dump on container using jmap gz=1 (JDK 17+) to reduce transfer size; output is .hprof.gz", + Type: typeBool, + }, } func (c *JavaPlugin) createOptionsParser() flags.FlagContext { @@ -350,6 +373,15 @@ func (c *JavaPlugin) parseOptions(args []string) (*Options, []string, error) { ContainerDir: commandFlags.String("container-dir"), LocalDir: commandFlags.String("local-dir"), Args: commandFlags.String("args"), + Redact: commandFlags.IsSet(flagRedact), + RedactComplete: commandFlags.IsSet(flagRedactComplete), + Compress: commandFlags.IsSet(flagCompress), + } + + if options.Redact && options.RedactComplete { + return nil, nil, &InvalidUsageError{ + message: "Error: flags '--redact' and '--redact-complete' are mutually exclusive", + } } return options, commandFlags.Args(), nil @@ -516,7 +548,7 @@ func (c *JavaPlugin) replaceVariables(command, appName, fspath, fileName, static var commands = []Command{ { - Name: "heap-dump", + Name: cmdHeapDump, Description: "Generate a heap dump from a running Java application", GenerateFiles: true, FileExtension: ".hprof", @@ -545,12 +577,12 @@ if [ -z "${JMAP_COMMAND}" ] && [ -z "${JVMMON_COMMAND}" ]; then buildpack: https://github.com/cloudfoundry/java-buildpack env: JBP_CONFIG_OPEN_JDK_JRE: '{ jre: { repository_root: "https://java-buildpack.cloudfoundry.org/openjdk-jdk/jammy/x86_64", version: 21.+ } }' - + " exit 1 fi if [ -n "${JMAP_COMMAND}" ]; then -OUTPUT=$( ${JMAP_COMMAND} -dump:format=b,file=@FILE_NAME $(pidof java) ) || STATUS_CODE=$? +OUTPUT=$( ${JMAP_COMMAND} -dump:format=b@JMAP_GZ,file=@FILE_NAME $(pidof java) ) || STATUS_CODE=$? if [ ! -s @FILE_NAME ]; then echo >&2 ${OUTPUT}; exit 1; fi if [ ${STATUS_CODE:-0} -gt 0 ]; then echo >&2 ${OUTPUT}; exit ${STATUS_CODE}; fi elif [ -n "${JVMMON_COMMAND}" ]; then @@ -561,6 +593,7 @@ HEAP_DUMP_NAME=$(find @FSPATH -name 'java_pid*.hprof' -printf '%T@ %p\0' | sort SIZE=-1; OLD_SIZE=$(stat -c '%s' "${HEAP_DUMP_NAME}"); while [ ${SIZE} != ${OLD_SIZE} ]; do OLD_SIZE=${SIZE}; sleep 3; SIZE=$(stat -c '%s' "${HEAP_DUMP_NAME}"); done if [ ! -s "${HEAP_DUMP_NAME}" ]; then echo >&2 ${OUTPUT}; exit 1; fi if [ ${STATUS_CODE:-0} -gt 0 ]; then echo >&2 ${OUTPUT}; exit ${STATUS_CODE}; fi +if [ -n "@COMPRESS_FLAG" ]; then gzip -1 "${HEAP_DUMP_NAME}" && HEAP_DUMP_NAME="${HEAP_DUMP_NAME}.gz"; fi fi`, FileLabel: "heap dump", FileNamePart: "heapdump", @@ -1066,6 +1099,10 @@ func (c *JavaPlugin) execute(_ plugin.CliConnection, args []string) (string, err fileName := "" staticFileName := "" fspath := remoteDir + fileExt := command.FileExtension + if command.Name == cmdHeapDump && options.Compress { + fileExt = ".hprof.gz" + } // Initialize fspath and fileName for commands that need them if command.GenerateFiles || command.NeedsFileName || command.GenerateArbitraryFiles { @@ -1098,13 +1135,23 @@ func (c *JavaPlugin) execute(_ plugin.CliConnection, args []string) (string, err if command.FileNamePart != "" { namePart = "-" + command.FileNamePart } - fileName = fspath + "/" + applicationName + namePart + "-" + utils.GenerateUUID() + command.FileExtension - staticFileName = fspath + "/" + applicationName + namePart + command.FileExtension + fileName = fspath + "/" + applicationName + namePart + "-" + utils.GenerateUUID() + fileExt + staticFileName = fspath + "/" + applicationName + namePart + fileExt c.logVerbosef("Generated filename: %s", fileName) c.logVerbosef("Generated static filename without UUID: %s", staticFileName) } commandText := command.SSHCommand + // Expand compress placeholders for heap-dump before the general variable substitution + if command.Name == cmdHeapDump { + if options.Compress { + commandText = strings.ReplaceAll(commandText, "@JMAP_GZ", ",gz=1") + commandText = strings.ReplaceAll(commandText, "@COMPRESS_FLAG", "1") + } else { + commandText = strings.ReplaceAll(commandText, "@JMAP_GZ", "") + commandText = strings.ReplaceAll(commandText, "@COMPRESS_FLAG", "") + } + } // Perform variable replacements directly in Go code var err2 error commandText, err2 = c.replaceVariables(commandText, applicationName, fspath, fileName, staticFileName, options.Args) @@ -1161,15 +1208,18 @@ func (c *JavaPlugin) execute(_ plugin.CliConnection, args []string) (string, err var finalFile string var err error - switch command.FileExtension { + switch fileExt { case ".hprof": c.logVerbosef("Finding heap dump file") finalFile, err = utils.FindHeapDumpFile(cfSSHArguments, fileName, fspath, applicationName+"-"+command.FileNamePart) + case ".hprof.gz": + c.logVerbosef("Finding compressed heap dump file") + finalFile, err = utils.FindHeapDumpGzFile(cfSSHArguments, fileName, fspath, applicationName+"-"+command.FileNamePart) case ".jfr": c.logVerbosef("Finding JFR file") finalFile, err = utils.FindJFRFile(cfSSHArguments, fileName, fspath, applicationName+"-"+command.FileNamePart) default: - return "", &InvalidUsageError{message: fmt.Sprintf("Unsupported file extension %q", command.FileExtension)} + return "", &InvalidUsageError{message: fmt.Sprintf("Unsupported file extension %q", fileExt)} } if err == nil && finalFile != "" { fileName = finalFile @@ -1189,12 +1239,29 @@ func (c *JavaPlugin) execute(_ plugin.CliConnection, args []string) (string, err return output, nil } - localFileFullPath := localDir + "/" + applicationName + "-" + command.FileNamePart + "-" + utils.GenerateUUID() + command.FileExtension + localFileFullPath := localDir + "/" + applicationName + "-" + command.FileNamePart + "-" + utils.GenerateUUID() + fileExt c.logVerbosef("Downloading file to: %s", localFileFullPath) err = utils.CopyOverCat(cfSSHArguments, fileName, localFileFullPath) if err == nil { c.logVerbosef("File download completed successfully") fmt.Println(utils.ToSentenceCase(command.FileLabel) + " file saved to: " + localFileFullPath) + + if command.Name == cmdHeapDump && (options.Redact || options.RedactComplete) { + mode := "lean" + if options.RedactComplete { + mode = "complete" + } + redactBin, rerr := ensureHprofRedact() + if rerr != nil { + return "", fmt.Errorf("hprof-redact unavailable: %w", rerr) + } + localIsGz := strings.HasSuffix(localFileFullPath, ".hprof.gz") + finalPath, rerr := pipeHeapDumpThroughRedact(redactBin, localFileFullPath, mode, options.Compress || localIsGz) + if rerr != nil { + return "", fmt.Errorf("redaction failed (unredacted file at %s): %w", localFileFullPath, rerr) + } + fmt.Println("Redacted heap dump saved to: " + finalPath) + } } else { c.logVerbosef("File download failed: %v", err) fmt.Fprintf(os.Stderr, "The %s was created successfully in the container at: %s\n", command.FileLabel, fileName) diff --git a/jstall.go b/jstall.go index e5ef136..4bc56be 100644 --- a/jstall.go +++ b/jstall.go @@ -25,7 +25,7 @@ import ( var jstallJarBytes []byte func javaExecutable() string { - if runtime.GOOS == "windows" { + if runtime.GOOS == osWindows { return "java.exe" } return cmdJava @@ -76,7 +76,7 @@ func platformJavaCandidates() []string { case "linux": matches, _ := filepath.Glob("/usr/lib/jvm/*/bin/" + exe) candidates = append(candidates, matches...) - case "windows": + case osWindows: for _, envVar := range []string{"ProgramFiles", "ProgramFiles(x86)", "ProgramW6432"} { base := os.Getenv(envVar) if base == "" { diff --git a/redact.go b/redact.go new file mode 100644 index 0000000..148620f --- /dev/null +++ b/redact.go @@ -0,0 +1,121 @@ +/* + * Copyright (c) 2024 SAP SE or an SAP affiliate company. All rights reserved. + * This file is licensed under the Apache Software License, v. 2 except as noted + * otherwise in the LICENSE file at the root of the repository. + */ + +package main + +import ( + "crypto/sha256" + _ "embed" + "encoding/hex" + "fmt" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" +) + +//go:embed dist/hprof-redact-linux-amd64 +var hprofRedactLinuxAmd64 []byte + +//go:embed dist/hprof-redact-linux-arm64 +var hprofRedactLinuxArm64 []byte + +//go:embed dist/hprof-redact-darwin-arm64 +var hprofRedactDarwinArm64 []byte + +//go:embed dist/hprof-redact-windows-amd64.exe +var hprofRedactWindowsAmd64 []byte + +// hprofRedactBytes returns the embedded hprof-redact binary for the current platform, +// or (nil, false) if this platform is not supported. +func hprofRedactBytes() ([]byte, bool) { + switch runtime.GOOS + "/" + runtime.GOARCH { + case "linux/amd64": + return hprofRedactLinuxAmd64, true + case "linux/arm64": + return hprofRedactLinuxArm64, true + case "darwin/arm64": + return hprofRedactDarwinArm64, true + case "windows/amd64": + return hprofRedactWindowsAmd64, true + default: + return nil, false + } +} + +// ensureHprofRedact extracts the embedded hprof-redact binary to the plugin cache +// directory (same location as jstall) and returns its path. +func ensureHprofRedact() (string, error) { + data, ok := hprofRedactBytes() + if !ok { + return "", fmt.Errorf("hprof-redact is not available for %s/%s; install manually: https://github.com/parttimenerd/hprof-analyzer/releases", runtime.GOOS, runtime.GOARCH) + } + + cacheDir, err := os.UserCacheDir() + if err != nil { + cacheDir = os.TempDir() + } + pluginCacheDir := filepath.Join(cacheDir, "cf-java-plugin") + if err := os.MkdirAll(pluginCacheDir, 0o755); err != nil { //nolint:gosec // 0755 is correct for a cache dir + return "", err + } + + h := sha256.Sum256(data) + hash := hex.EncodeToString(h[:8]) + binPath := filepath.Join(pluginCacheDir, fmt.Sprintf("hprof-redact-%s", hash)) + if runtime.GOOS == osWindows { + binPath += ".exe" + } + + // Re-use if already extracted + if _, err := os.Stat(binPath); err == nil { + return binPath, nil + } + + if err := os.WriteFile(binPath, data, 0o755); err != nil { //nolint:gosec // 0755: binary must be executable + return "", fmt.Errorf("failed to extract hprof-redact: %w", err) + } + return binPath, nil +} + +// pipeHeapDumpThroughRedact runs hprof-redact on localPath, writing the output +// to a new file derived from localPath. On success it deletes the original +// unredacted file and returns the path to the redacted file. +// +// mode must be "lean" or "complete". If compress is true the output is written +// as .hprof.gz. +func pipeHeapDumpThroughRedact(redactBin, localPath, mode string, compress bool) (string, error) { + base := strings.TrimSuffix(localPath, ".hprof.gz") + base = strings.TrimSuffix(base, ".hprof") + var outputPath string + if compress { + outputPath = base + "-redacted.hprof.gz" + } else { + outputPath = base + "-redacted.hprof" + } + + var args []string + if mode == "complete" { + args = append(args, "--complete") + } + args = append(args, localPath, outputPath) + + cmd := exec.Command(redactBin, args...) //nolint:gosec // redactBin comes from ensureHprofRedact, not user input + cmd.Stderr = os.Stderr + + if err := cmd.Run(); err != nil { + return "", fmt.Errorf("hprof-redact failed: %w", err) + } + + // Remove the unredacted source file + if err := os.Remove(localPath); err != nil { + // Non-fatal: redacted file is already written + fmt.Fprintf(os.Stderr, "warning: could not remove unredacted file %s: %v\n", localPath, err) + } + + return outputPath, nil +} diff --git a/utils/cfutils.go b/utils/cfutils.go index dbde266..e9ccb74 100644 --- a/utils/cfutils.go +++ b/utils/cfutils.go @@ -262,6 +262,11 @@ func FindHeapDumpFile(args []string, fullpath string, fspath string, namePrefix return FindFile(args, fullpath, fspath, "*.hprof", namePrefix) } +// FindHeapDumpGzFile locates gzip-compressed heap dump files (*.hprof.gz) on the remote container. +func FindHeapDumpGzFile(args []string, fullpath string, fspath string, namePrefix string) (string, error) { + return FindFile(args, fullpath, fspath, "*.hprof.gz", namePrefix) +} + // FindJFRFile locates Java Flight Recorder files (*.jfr) in the specified path on the remote container. func FindJFRFile(args []string, fullpath string, fspath string, namePrefix string) (string, error) { return FindFile(args, fullpath, fspath, "*.jfr", namePrefix) From 00672a09b01727a2e5fff5e86a41a8b1a7f7a331 Mon Sep 17 00:00:00 2001 From: Johannes Bechberger Date: Wed, 9 Sep 2026 14:45:18 +0200 Subject: [PATCH 02/39] feat: Windows ARM support + default gz compression for heap-dump MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - hprof-analyzer release.yml: add aarch64-pc-windows-msvc target on windows-11-arm runner; compile-all includes GOOS=windows GOARCH=arm64 - redact.go: embed dist/hprof-redact-windows-arm64.exe (stub until v0.3.1 ships); add windows/arm64 case; guard against 0-byte stubs - cfutils.go: add CopyOverCatGunzip (io.Pipe + compress/gzip for transparent streaming decompression) and ProbeRemoteFileGzip (checks gzip magic bytes 1f8b via SSH) - cf_cli_java_plugin.go: heap-dump SSHCommand now uses shell-level gz probe (jmap -h | grep gz) so jmap auto-uses gz=1 on JDK 17+; Go post-command probes the remote file magic bytes to decide: - remote gz + no --compress → CopyOverCatGunzip, save as .hprof - remote gz + --compress → CopyOverCat, save as .hprof.gz - not gz + --compress → warn JDK 17+ required, save uncompressed Removes @JMAP_GZ Go expansion (replaced by shell probe); keeps @COMPRESS_FLAG for jvmmon path --- Makefile | 13 +++++++++- cf_cli_java_plugin.go | 45 ++++++++++++++++++++++++++------- redact.go | 10 +++++++- utils/cfutils.go | 59 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 116 insertions(+), 11 deletions(-) diff --git a/Makefile b/Makefile index 56ef2c2..14eb27d 100644 --- a/Makefile +++ b/Makefile @@ -66,11 +66,21 @@ dist/hprof-redact-windows-amd64.exe: cp $(WINTMP)/hprof-analyzer-x86_64-pc-windows-msvc/hprof-redact.exe $@ rm -rf $(WINTMP) +dist/hprof-redact-windows-arm64.exe: + mkdir -p dist + $(eval WINTMP := $(shell mktemp -d)) + curl -sL -f $(HPROF_REDACT_BASE)/hprof-analyzer-aarch64-pc-windows-msvc.zip -o $(WINTMP)/win.zip \ + && cd $(WINTMP) && unzip -o win.zip hprof-analyzer-aarch64-pc-windows-msvc/hprof-redact.exe \ + && cp $(WINTMP)/hprof-analyzer-aarch64-pc-windows-msvc/hprof-redact.exe $@ \ + || touch $@ + rm -rf $(WINTMP) + HPROF_REDACT_BINS = \ dist/hprof-redact-linux-amd64 \ dist/hprof-redact-linux-arm64 \ dist/hprof-redact-darwin-arm64 \ - dist/hprof-redact-windows-amd64.exe + dist/hprof-redact-windows-amd64.exe \ + dist/hprof-redact-windows-arm64.exe download-hprof-redact: $(HPROF_REDACT_BINS) @@ -86,6 +96,7 @@ compile-all: $(JSTALL_DEP) $(HPROF_REDACT_BINS) GOOS=linux GOARCH=arm64 go build -o build/cf-cli-java-plugin-linux-arm64 . GOOS=darwin GOARCH=arm64 go build -o build/cf-cli-java-plugin-osx-arm64 . GOOS=windows GOARCH=amd64 go build -o build/cf-cli-java-plugin-win64.exe . + GOOS=windows GOARCH=arm64 go build -o build/cf-cli-java-plugin-win-arm64.exe . clean: rm -r build diff --git a/cf_cli_java_plugin.go b/cf_cli_java_plugin.go index 02b0118..063f1b4 100644 --- a/cf_cli_java_plugin.go +++ b/cf_cli_java_plugin.go @@ -49,6 +49,7 @@ const ( extJFR = ".jfr" labelJFR = "JFR recording" partJFR = "jfr" + extHprof = ".hprof" ) // JavaPlugin is a CF CLI plugin that supports taking heap and thread dumps on demand @@ -551,7 +552,7 @@ var commands = []Command{ Name: cmdHeapDump, Description: "Generate a heap dump from a running Java application", GenerateFiles: true, - FileExtension: ".hprof", + FileExtension: extHprof, /* If there is not enough space on the filesystem to write the dump, jmap will create a file with size 0, output something about not enough space left on the device, and exit with status code 0. @@ -582,7 +583,9 @@ if [ -z "${JMAP_COMMAND}" ] && [ -z "${JVMMON_COMMAND}" ]; then exit 1 fi if [ -n "${JMAP_COMMAND}" ]; then -OUTPUT=$( ${JMAP_COMMAND} -dump:format=b@JMAP_GZ,file=@FILE_NAME $(pidof java) ) || STATUS_CODE=$? +GZ_ARG="" +if ${JMAP_COMMAND} -h 2>&1 | grep -q "gz="; then GZ_ARG=",gz=1"; fi +OUTPUT=$( ${JMAP_COMMAND} -dump:format=b${GZ_ARG},file=@FILE_NAME $(pidof java) ) || STATUS_CODE=$? if [ ! -s @FILE_NAME ]; then echo >&2 ${OUTPUT}; exit 1; fi if [ ${STATUS_CODE:-0} -gt 0 ]; then echo >&2 ${OUTPUT}; exit ${STATUS_CODE}; fi elif [ -n "${JVMMON_COMMAND}" ]; then @@ -1101,7 +1104,8 @@ func (c *JavaPlugin) execute(_ plugin.CliConnection, args []string) (string, err fspath := remoteDir fileExt := command.FileExtension if command.Name == cmdHeapDump && options.Compress { - fileExt = ".hprof.gz" + // Only set .hprof.gz for jvmmon path (explicit compress); jmap always writes .hprof on remote + fileExt = extHprof } // Initialize fspath and fileName for commands that need them @@ -1142,13 +1146,11 @@ func (c *JavaPlugin) execute(_ plugin.CliConnection, args []string) (string, err } commandText := command.SSHCommand - // Expand compress placeholders for heap-dump before the general variable substitution + // Expand @COMPRESS_FLAG for jvmmon path in heap-dump (jmap uses shell-level gz probe) if command.Name == cmdHeapDump { if options.Compress { - commandText = strings.ReplaceAll(commandText, "@JMAP_GZ", ",gz=1") commandText = strings.ReplaceAll(commandText, "@COMPRESS_FLAG", "1") } else { - commandText = strings.ReplaceAll(commandText, "@JMAP_GZ", "") commandText = strings.ReplaceAll(commandText, "@COMPRESS_FLAG", "") } } @@ -1209,7 +1211,7 @@ func (c *JavaPlugin) execute(_ plugin.CliConnection, args []string) (string, err var finalFile string var err error switch fileExt { - case ".hprof": + case extHprof: c.logVerbosef("Finding heap dump file") finalFile, err = utils.FindHeapDumpFile(cfSSHArguments, fileName, fspath, applicationName+"-"+command.FileNamePart) case ".hprof.gz": @@ -1239,9 +1241,34 @@ func (c *JavaPlugin) execute(_ plugin.CliConnection, args []string) (string, err return output, nil } - localFileFullPath := localDir + "/" + applicationName + "-" + command.FileNamePart + "-" + utils.GenerateUUID() + fileExt + // For heap-dump via jmap: probe whether the remote file is gzip-compressed. + // jmap writes a .hprof filename but may fill it with gzip content when gz=1 is supported. + localFileExt := fileExt + remoteIsGz := false + if command.Name == cmdHeapDump && fileExt == extHprof { + remoteIsGz, _ = utils.ProbeRemoteFileGzip(cfSSHArguments, fileName) + c.logVerbosef("Remote file is gzip-compressed: %t", remoteIsGz) + switch { + case remoteIsGz && options.Compress: + // User asked for .hprof.gz locally → keep compressed + localFileExt = ".hprof.gz" + case !remoteIsGz && options.Compress: + fmt.Fprintf(os.Stderr, "Warning: remote jmap does not support gz compression (JDK 17+ required); downloading uncompressed\n") + case remoteIsGz: + fmt.Println("Note: remote jmap used gz compression; decompressing during transfer...") + } + } + + localFileFullPath := localDir + "/" + applicationName + "-" + command.FileNamePart + "-" + utils.GenerateUUID() + localFileExt c.logVerbosef("Downloading file to: %s", localFileFullPath) - err = utils.CopyOverCat(cfSSHArguments, fileName, localFileFullPath) + + if command.Name == cmdHeapDump && remoteIsGz && localFileExt == extHprof { + // Transparent decompression: stream gz from remote, write plain .hprof locally + err = utils.CopyOverCatGunzip(cfSSHArguments, fileName, localFileFullPath) + } else { + err = utils.CopyOverCat(cfSSHArguments, fileName, localFileFullPath) + } + if err == nil { c.logVerbosef("File download completed successfully") fmt.Println(utils.ToSentenceCase(command.FileLabel) + " file saved to: " + localFileFullPath) diff --git a/redact.go b/redact.go index 148620f..246a5b7 100644 --- a/redact.go +++ b/redact.go @@ -30,6 +30,9 @@ var hprofRedactDarwinArm64 []byte //go:embed dist/hprof-redact-windows-amd64.exe var hprofRedactWindowsAmd64 []byte +//go:embed dist/hprof-redact-windows-arm64.exe +var hprofRedactWindowsArm64 []byte + // hprofRedactBytes returns the embedded hprof-redact binary for the current platform, // or (nil, false) if this platform is not supported. func hprofRedactBytes() ([]byte, bool) { @@ -40,8 +43,10 @@ func hprofRedactBytes() ([]byte, bool) { return hprofRedactLinuxArm64, true case "darwin/arm64": return hprofRedactDarwinArm64, true - case "windows/amd64": + case osWindows + "/amd64": return hprofRedactWindowsAmd64, true + case osWindows + "/arm64": + return hprofRedactWindowsArm64, true default: return nil, false } @@ -54,6 +59,9 @@ func ensureHprofRedact() (string, error) { if !ok { return "", fmt.Errorf("hprof-redact is not available for %s/%s; install manually: https://github.com/parttimenerd/hprof-analyzer/releases", runtime.GOOS, runtime.GOARCH) } + if len(data) == 0 { + return "", fmt.Errorf("hprof-redact for %s/%s was not available at build time; install manually: https://github.com/parttimenerd/hprof-analyzer/releases", runtime.GOOS, runtime.GOARCH) + } cacheDir, err := os.UserCacheDir() if err != nil { diff --git a/utils/cfutils.go b/utils/cfutils.go index e9ccb74..171f0b8 100644 --- a/utils/cfutils.go +++ b/utils/cfutils.go @@ -2,11 +2,13 @@ package utils import ( + "compress/gzip" "crypto/rand" "encoding/hex" "encoding/json" "errors" "fmt" + "io" "os" "os/exec" "path/filepath" @@ -246,6 +248,63 @@ func CopyOverCat(args []string, src string, dest string) error { return nil } +// CopyOverCatGunzip streams a remote gzip-compressed file via cf ssh and decompresses +// it on the fly, saving the result at dest. +func CopyOverCatGunzip(args []string, src string, dest string) error { + if dir := filepath.Dir(dest); dir != "" && dir != "." { + if err := os.MkdirAll(dir, 0o755); err != nil { //nolint:gosec // 0755 is correct for a local download directory + return fmt.Errorf("cannot create local directory %s: %w", dir, err) + } + } + f, err := os.OpenFile(dest, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o600) //nolint:gosec // dest is a plugin-constructed output path + if err != nil { + return errors.New("Error creating local file at " + dest + ". Please check that you are allowed to create files at the given local path.") + } + defer func() { + if closeErr := f.Close(); closeErr != nil { + fmt.Fprintf(os.Stderr, "Warning: failed to close file %s: %v\n", dest, closeErr) + } + }() + + pr, pw := io.Pipe() + catArgs := append(args, "cat \""+src+"\"") //nolint:gocritic // intentional new slice + cat := exec.Command("cf", catArgs...) + cat.Stdout = pw + + if err := cat.Start(); err != nil { + _ = pr.Close() + _ = pw.Close() + return errors.New("error starting cf ssh: " + err.Error()) + } + + go func() { + _ = pw.CloseWithError(cat.Wait()) + }() + + gz, err := gzip.NewReader(pr) + if err != nil { + return fmt.Errorf("gzip header error: %w", err) + } + defer func() { _ = gz.Close() }() + + if _, err := io.Copy(f, gz); err != nil { //nolint:gosec // G110: source is a trusted CF container owned by the user + return fmt.Errorf("decompression failed: %w", err) + } + return nil +} + +// ProbeRemoteFileGzip checks if the first 2 bytes of a remote file are the gzip magic bytes (1f 8b). +func ProbeRemoteFileGzip(args []string, path string) (bool, error) { + cmd := fmt.Sprintf("xxd -l 2 \"%s\" 2>/dev/null || od -An -N2 -tx1 \"%s\" 2>/dev/null", path, path) + probeArgs := append(args, cmd) //nolint:gocritic // intentional new slice + out, err := exec.Command("cf", probeArgs...).Output() + if err != nil { + return false, err + } + outStr := string(out) + return strings.Contains(outStr, "1f") && strings.Contains(outStr, "8b"), nil +} + // DeleteRemoteFile removes a file from the remote Cloud Foundry application container. func DeleteRemoteFile(args []string, path string) error { args = append(args, "rm -fr \""+path+"\"") From 08c93c12679eb3382f75ec6415b313efc682fc6c Mon Sep 17 00:00:00 2001 From: Johannes Bechberger Date: Wed, 9 Sep 2026 15:14:06 +0200 Subject: [PATCH 03/39] fix: dry-run bypass CheckRequiredTools + appInstanceIndex detection - appInstanceIndexSet: simonleung8/flags IsSet() returns true for any registered flag whose non-zero default was set at init time, making it impossible to distinguish user-provided from default. Compare against known default (-1) instead. - CheckRequiredTools wrapped in !options.DryRun guard so dry-run works without CF login or SSH access. - var err declaration hoisted before GenerateFiles block (needed after CheckRequiredTools was scoped inside the if block). --- cf_cli_java_plugin.go | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/cf_cli_java_plugin.go b/cf_cli_java_plugin.go index 063f1b4..ff38a4a 100644 --- a/cf_cli_java_plugin.go +++ b/cf_cli_java_plugin.go @@ -339,7 +339,10 @@ func (c *JavaPlugin) parseOptions(args []string) (*Options, []string, error) { } appInstanceIndex := commandFlags.Int("app-instance-index") - appInstanceIndexSet := commandFlags.IsSet("app-instance-index") + // simonleung8/flags registers flags with non-zero defaults in flagsets at init time, + // so IsSet() returns true even when the flag was not explicitly provided. + // Check against the known default (-1) to detect actual user-provided values. + appInstanceIndexSet := commandFlags.IsSet("app-instance-index") && appInstanceIndex != -1 keep := commandFlags.IsSet("keep") noDownload := commandFlags.IsSet("no-download") @@ -1021,13 +1024,15 @@ func (c *JavaPlugin) execute(_ plugin.CliConnection, args []string) (string, err c.logVerbosef("CF SSH arguments: %v", cfSSHArguments) - supported, err := utils.CheckRequiredTools(applicationName) + if !options.DryRun { + supported, err := utils.CheckRequiredTools(applicationName) - if err != nil || !supported { - return "required tools checking failed", err - } + if err != nil || !supported { + return "required tools checking failed", err + } - c.logVerbosef("Required tools check passed") + c.logVerbosef("Required tools check passed") + } if command.IsLocal { c.logVerbosef("Executing local command: %s", command.Name) @@ -1103,6 +1108,7 @@ func (c *JavaPlugin) execute(_ plugin.CliConnection, args []string) (string, err staticFileName := "" fspath := remoteDir fileExt := command.FileExtension + var err error if command.Name == cmdHeapDump && options.Compress { // Only set .hprof.gz for jvmmon path (explicit compress); jmap always writes .hprof on remote fileExt = extHprof From a1ada44bd6cfc9f9766c1fc1d4b1092b4651435f Mon Sep 17 00:00:00 2001 From: Johannes Bechberger Date: Wed, 9 Sep 2026 15:18:35 +0200 Subject: [PATCH 04/39] docs: document --redact, --redact-complete, --compress flags for heap-dump - Feature list in intro - Examples: --redact, --redact-complete, --compress, combined usage - New "Heap Dump Privacy" subsection: redaction modes table, what is preserved, file naming, supported platforms - New "Compressed Transfer" subsection: JDK 17+ requirement, fallback behaviour, transparent gz transfer without --compress - CHANGELOG [Unreleased]: all four new behaviours --- CHANGELOG.md | 12 +++++-- README.md | 94 ++++++++++++++++++++++++++++++++++++---------------- 2 files changed, 76 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 20831d2..c2a87a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,12 +10,20 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ### Added - Bundle [jstall](https://github.com/parttimenerd/jstall) (jstall-minimal.jar) for one-shot JVM inspection via - `cf java jstall APP_NAME`. Requires Java 17+ locally. Supports all jstall subcommands via `jstall APP --args`. + `cf java jstall APP_NAME`. Requires Java 17+ locally. Supports all jstall subcommands via `--args`. +- `heap-dump --redact`: zeros primitive arrays (`byte[]`, `char[]`, etc.) in the downloaded dump before saving + (lean redaction mode), using the bundled [hprof-redact](https://github.com/parttimenerd/hprof-analyzer) binary. + Supported on Linux, macOS (Apple Silicon), and Windows. +- `heap-dump --redact-complete`: zeros all primitive arrays and individual primitive fields (complete redaction mode, + maximum privacy). Mutually exclusive with `--redact`. +- `heap-dump --compress`: saves the dump as `.hprof.gz` by transferring it gzip-compressed over SSH (requires JDK 17+ + on the container). Prints a warning and falls back to uncompressed on older JDKs. +- Transparent compressed transfer: on JDK 17+ containers, the plugin automatically uses `jmap gz=1` to reduce + transfer size even without `--compress`, decompressing on the fly so the local file is always a plain `.hprof`. ### Changed - Improved SSH error messages for better clarity and debugging -- Enhanced documentation and README with better clarity ## [4.0.2] diff --git a/README.md b/README.md index 3e9e15c..fdac700 100644 --- a/README.md +++ b/README.md @@ -9,12 +9,14 @@ work with Java applications deployed on Cloud Foundry by the [SapMachine](https: Currently, it allows you to: -- Trigger and retrieve a heap dump and a thread dump from a Cloud Foundry Java application -- Run jcmd remotely on your application -- Start, stop and retrieve JFR and [async-profiler](https://github.com/jvm-profiling-tools/async-profiler) - ([SapMachine](https://sapmachine.io) only) profiles from your application +- Capture heap dumps and thread dumps from a running Cloud Foundry Java application +- Run `jcmd` remotely against your application +- Start, stop, and retrieve JFR and [async-profiler](https://github.com/jvm-profiling-tools/async-profiler) + ([SapMachine](https://sapmachine.io) only) profiles - Run [jstall](https://github.com/parttimenerd/jstall) for one-shot JVM inspection (deadlock detection, hot threads, dependency graphs, and more): bundled directly in the plugin, requires Java 17+ locally +- Redact heap dumps before saving to remove sensitive data (`--redact`, `--redact-complete`) +- Reduce transfer size by compressing heap dumps over SSH (`--compress`) ## Installation @@ -124,43 +126,47 @@ is not in `cf java`, but in whatever makes `cf ssh` fail. ### Examples -Getting a heap-dump: +Getting a heap dump: ```sh -> cf java heap-dump $APP_NAME --> ./$APP_NAME-heapdump-$RANDOM.hprof +# Basic — plain .hprof saved locally +cf java heap-dump $APP_NAME + +# Redact sensitive values (passwords, tokens, personal data) before saving +cf java heap-dump $APP_NAME --redact # lean: zeros primitive arrays +cf java heap-dump $APP_NAME --redact-complete # complete: zeros all primitive values + +# Compress the output (JDK 17+ on container required; falls back to uncompressed otherwise) +cf java heap-dump $APP_NAME --compress # saves as .hprof.gz + +# Redact and compress +cf java heap-dump $APP_NAME --redact --compress ``` -Getting a thread-dump: +Getting a thread dump: ```sh -> cf java thread-dump $APP_NAME -... -Full thread dump OpenJDK 64-Bit Server VM ... -... +cf java thread-dump $APP_NAME ``` -Creating a CPU-time profile via async-profiler: +Creating a CPU profile via async-profiler: ```sh -> cf java asprof-start-cpu $APP_NAME -Profiling started +cf java asprof-start-cpu $APP_NAME # wait some time to gather data -> cf java asprof-stop $APP_NAME --> ./$APP_NAME-asprof-$RANDOM.jfr +cf java asprof-stop $APP_NAME ``` -Running arbitrary JCMD commands, like `VM.uptime`: +Running arbitrary jcmd commands, like `VM.uptime`: ```sh -> cf java jcmd $APP_NAME --args 'VM.uptime' -$TIME s +cf java jcmd $APP_NAME --args 'VM.uptime' ``` Quick status check of the remote JVM (requires Java 17+ locally): ```sh -> cf java status $APP_NAME +cf java status $APP_NAME ``` Running [JStall](https://github.com/parttimenerd/jstall) for more specific JVM inspection (requires Java 17+ locally): @@ -245,27 +251,59 @@ The `--args` parameter passes values directly into remote shell commands via `cf shell features like environment variable expansion and piping. **Do not pass untrusted input to `--args`** — treat it with the same caution as a shell command. +### File Output + The heap dumps and profiles will be downloaded to a local file automatically (to the current directory by default). Use `--local-dir` to specify a different download location. To save disk space of the application container, the files are automatically deleted unless the `--keep` option is set. -Providing `--container-dir` is optional. If specified the plugin will create the heap dump or profile at the given file -path in the application container. Without providing this parameter, the file will be created either at `/tmp` or at the -file path of a file system service if attached to the container. +Providing `--container-dir` is optional. If specified, the plugin will create the heap dump or profile at that path +inside the application container. Without it, the file is created at `/tmp` or at the mount point of an attached +file system service. ```shell cf java [heap-dump|jfr-stop|jfr-dump|asprof-stop] [my-app] --local-dir /local/path [--container-dir /var/fspath] ``` -Everything else, like thread dumps, will be output to `std-out`. You may want to redirect the command's output to file, -e.g., by executing: +Thread dumps are streamed to stdout. To save one to a file: ```shell cf java thread-dump [my_app] -i [my_instance_index] > thread-dump.txt ``` -The `--keep` flag is invalid when invoking non file producing commands. (Unlike with heap dumps, the JVM does not need -to output the thread dump to file before streaming it out.) +The `--keep` flag is not applicable to commands that stream output directly (e.g., `thread-dump`). + +### Heap Dump Privacy + +Heap dumps contain the full in-memory state of a JVM, including strings, byte arrays, and field values, which can +hold passwords, tokens, session data, or personal information. Before sharing a dump outside a trusted environment, +use `--redact` or `--redact-complete` to zero out sensitive values. + +| Flag | What gets zeroed | +|------|-----------------| +| `--redact` | Primitive arrays (`byte[]`, `char[]`, `int[]`, …) — covers most strings and serialized data | +| `--redact-complete` | All primitive arrays **and** individual primitive fields — maximum privacy | + +Both modes preserve the full object graph (class names, references, instance counts), so the dump remains useful for +memory analysis. The two flags are mutually exclusive. + +The redacted file is saved locally with a `-redacted` suffix; the original unredacted file is deleted automatically. +Use `--redact --compress` to also compress the output (produces a `.hprof.gz`). + +Redaction runs locally via the bundled [hprof-redact](https://github.com/parttimenerd/hprof-analyzer) binary after +the dump is downloaded. Supported platforms: Linux (x86_64, arm64), macOS (Apple Silicon), Windows (x86_64, arm64). + +### Compressed Transfer + +When bandwidth or container disk space is a concern, use `--compress` to transfer the dump in gzip format. + +- On **JDK 17+**: `jmap` compresses the dump on the container before transfer; the local file is saved as `.hprof.gz`. +- On **JDK < 17**: the container JDK does not support `gz=1`; a warning is printed and the dump is downloaded + uncompressed as usual. + +Without `--compress`, the plugin still uses `gz=1` automatically when the remote JDK supports it — the transfer is +compressed but the local file is transparently decompressed to a plain `.hprof`. This is the default behaviour +starting from JDK 17 and costs nothing from the user's perspective. ## Limitations From 10203963bbfdc20c5d1dd2fe4bf0edd299f4c2aa Mon Sep 17 00:00:00 2001 From: Johannes Bechberger Date: Thu, 17 Sep 2026 13:46:37 +0200 Subject: [PATCH 05/39] docs: design spec for --open flag on heap-dump --- .../specs/2026-09-17-open-heap-dump-design.md | 86 +++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100644 docs/superpowers/specs/2026-09-17-open-heap-dump-design.md diff --git a/docs/superpowers/specs/2026-09-17-open-heap-dump-design.md b/docs/superpowers/specs/2026-09-17-open-heap-dump-design.md new file mode 100644 index 0000000..40eb0ed --- /dev/null +++ b/docs/superpowers/specs/2026-09-17-open-heap-dump-design.md @@ -0,0 +1,86 @@ +# Design: `--open` flag for heap-dump + +**Date:** 2026-09-17 +**Branch:** heap-dump-compress-redact +**Status:** Approved + +## Summary + +Add `--open` to the `heap-dump` command. After the final local file is saved (including any redaction or compression), the plugin spins up a temporary single-serve HTTP server, then opens the browser to the hprof-analyzer web app with a `?file=` URL pointing at that server. The server shuts down after the browser fetches the file once. + +The hprof-analyzer web app (`parttimenerd/hprof-analyzer`) is being updated in parallel to support `?file=URL` loading. + +## Flags + +| Flag | Type | Default | Description | +|------|------|---------|-------------| +| `--open` | bool | false | After download, open the heap dump in the hprof-analyzer web app | +| `--open-url` | string | `https://parttimenerd.github.io/hprof-analyzer` | Base URL of the hprof-analyzer web instance to open | + +**Validation rules:** +- `--open` is only valid with `heap-dump`; all other commands error at parse time +- `--open-url` implies `--open` +- `--open` + `--no-download` errors at parse time (no local file to serve) +- `--open` is compatible with `--redact`, `--redact-complete`, `--compress` — always opens the final output file + +## Options struct changes + +```go +// in Options struct +Open bool +OpenURL string +``` + +`OpenURL` defaults to `"https://parttimenerd.github.io/hprof-analyzer"` always (set at options-parse time, not conditionally on `--open`), so `--open-url` alone works without requiring `--open` to be explicitly set. + +## Execution flow + +After `localFileFullPath` is finalized (post-download, post-redact/compress): + +1. Bind `net/http` listener on `:0` (OS assigns free port) +2. Register a single handler that: + - Sets `Access-Control-Allow-Origin: *` (cross-origin fetch from the web app) + - Sets `Content-Type: application/octet-stream` + - Streams the file + - Signals a `done` channel after the response is written +3. Open browser to `{OpenURL}/?file=http://localhost:{PORT}/{basename}` + - macOS: `open ` + - Linux: `xdg-open ` + - Windows: `start ` +4. Block until `done` fires, then shut down the server and return + +## New file: `open.go` + +Two functions: + +```go +// serveFileOnce binds a random port, serves path exactly once, returns the +// port and a channel that closes when the request completes. +func serveFileOnce(path string) (port int, done <-chan struct{}, err error) + +// openBrowser opens url in the default system browser. +// If the browser cannot be launched, it prints the URL to stdout instead. +func openBrowser(url string) error +``` + +## Dry-run behaviour + +Print the URL that would be opened with a `PORT` placeholder instead of a real port: + +``` +Would open: https://parttimenerd.github.io/hprof-analyzer/?file=http://localhost:PORT/sapmachine21-heapdump-abc123.hprof +``` + +## Error handling + +| Scenario | Behaviour | +|----------|-----------| +| Port binding fails | Return error, abort command | +| Browser launch fails | Print URL to stdout, continue blocking for fetch | +| User Ctrl+Cs before fetch | Normal signal handling; plugin exits, server stops | +| `--open` + `--no-download` | Error at parse time | +| `--open` on non-heap-dump command | Error at parse time | + +## Dependencies + +No new external dependencies. Uses `net/http`, `os/exec`, `runtime` from stdlib. From a40dc1c212acbeac1748e2b945c01eb1a01b2bac Mon Sep 17 00:00:00 2001 From: Johannes Bechberger Date: Thu, 17 Sep 2026 13:51:21 +0200 Subject: [PATCH 06/39] docs: implementation plan for --open heap-dump flag --- .../plans/2026-09-17-open-heap-dump.md | 590 ++++++++++++++++++ 1 file changed, 590 insertions(+) create mode 100644 docs/superpowers/plans/2026-09-17-open-heap-dump.md diff --git a/docs/superpowers/plans/2026-09-17-open-heap-dump.md b/docs/superpowers/plans/2026-09-17-open-heap-dump.md new file mode 100644 index 0000000..6facd7a --- /dev/null +++ b/docs/superpowers/plans/2026-09-17-open-heap-dump.md @@ -0,0 +1,590 @@ +# `--open` Flag for heap-dump Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add `--open` and `--open-url` flags to `heap-dump` that spin up a single-serve local HTTP server and open the hprof-analyzer web app with `?file=http://localhost:PORT/dump.hprof` after the final local file is saved. + +**Architecture:** A new `open.go` file provides two functions — `serveFileOnce` (binds `:0`, serves the file once, returns port + done channel) and `openBrowser` (cross-platform browser launch). The main plugin wiring in `cf_cli_java_plugin.go` adds two flags, two `Options` fields, parse-time validation, and a call to the open logic after `localFileFullPath` is finalized. + +**Tech Stack:** Go stdlib (`net/http`, `os/exec`, `runtime`, `path/filepath`). No new dependencies. + +--- + +## File Map + +| File | Action | Responsibility | +|------|--------|----------------| +| `open.go` | **Create** | `serveFileOnce` + `openBrowser` | +| `cf_cli_java_plugin.go` | **Modify** | Flag constants, `Options` fields, `flagDefinitions` entries, `parseOptions` wiring + validation, post-download `--open` call | + +--- + +### Task 1: `open.go` — `serveFileOnce` and `openBrowser` + +**Files:** +- Create: `open.go` + +- [ ] **Step 1: Write the failing test** + +Create `open_test.go`: + +```go +package main + +import ( + "fmt" + "io" + "net/http" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestServeFileOnce(t *testing.T) { + // Create a temp file with known content + tmp := t.TempDir() + p := filepath.Join(tmp, "test.hprof") + if err := os.WriteFile(p, []byte("HEAP_CONTENT"), 0o600); err != nil { + t.Fatal(err) + } + + port, done, err := serveFileOnce(p) + if err != nil { + t.Fatalf("serveFileOnce: %v", err) + } + if port <= 0 { + t.Fatalf("expected positive port, got %d", port) + } + + url := fmt.Sprintf("http://localhost:%d/test.hprof", port) + resp, err := http.Get(url) //nolint:noctx + if err != nil { + t.Fatalf("GET %s: %v", url, err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + t.Fatalf("expected 200, got %d", resp.StatusCode) + } + if ct := resp.Header.Get("Content-Type"); ct != "application/octet-stream" { + t.Errorf("Content-Type: want application/octet-stream, got %s", ct) + } + if acao := resp.Header.Get("Access-Control-Allow-Origin"); acao != "*" { + t.Errorf("Access-Control-Allow-Origin: want *, got %s", acao) + } + body, _ := io.ReadAll(resp.Body) + if string(body) != "HEAP_CONTENT" { + t.Errorf("body: want HEAP_CONTENT, got %s", body) + } + + // done channel must close after the request completes + select { + case <-done: + default: + t.Error("done channel not closed after successful GET") + } +} + +func TestServeFileOnce_MissingFile(t *testing.T) { + _, _, err := serveFileOnce("/nonexistent/path/dump.hprof") + if err == nil { + t.Fatal("expected error for missing file, got nil") + } +} + +func TestBuildOpenURL(t *testing.T) { + cases := []struct { + base string + port int + filename string + want string + }{ + { + "https://parttimenerd.github.io/hprof-analyzer", + 54321, + "myapp-heapdump-abc.hprof", + "https://parttimenerd.github.io/hprof-analyzer/?file=http://localhost:54321/myapp-heapdump-abc.hprof", + }, + { + "https://parttimenerd.github.io/hprof-analyzer/", + 9000, + "dump.hprof.gz", + "https://parttimenerd.github.io/hprof-analyzer/?file=http://localhost:9000/dump.hprof.gz", + }, + } + for _, tc := range cases { + got := buildOpenURL(tc.base, tc.port, tc.filename) + if got != tc.want { + t.Errorf("buildOpenURL(%q, %d, %q)\n want %q\n got %q", tc.base, tc.port, tc.filename, tc.want, got) + } + } +} + +func TestBuildOpenURL_TrailingSlash(t *testing.T) { + // base with trailing slash must not produce double slash before ? + url := buildOpenURL("https://example.com/analyzer/", 1234, "dump.hprof") + if strings.Contains(url, "//?" ) { + t.Errorf("double slash before ?: %s", url) + } +} +``` + +- [ ] **Step 2: Run tests to verify they fail** + +```bash +go test -run 'TestServeFileOnce|TestBuildOpenURL' ./... 2>&1 +``` + +Expected: `undefined: serveFileOnce`, `undefined: buildOpenURL` + +- [ ] **Step 3: Implement `open.go`** + +```go +/* + * Copyright (c) 2024 SAP SE or an SAP affiliate company. All rights reserved. + * This file is licensed under the Apache Software License, v. 2 except as noted + * otherwise in the LICENSE file at the root of the repository. + */ + +package main + +import ( + "context" + "fmt" + "net" + "net/http" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" +) + +// serveFileOnce starts a local HTTP server on a random port that serves path +// exactly once. It returns the bound port and a channel that is closed when +// the first GET request completes. The server shuts itself down after serving. +// Returns an error if the file does not exist or the port cannot be bound. +func serveFileOnce(path string) (int, <-chan struct{}, error) { + if _, err := os.Stat(path); err != nil { + return 0, nil, fmt.Errorf("file not found: %w", err) + } + + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + return 0, nil, fmt.Errorf("could not bind local port: %w", err) + } + port := ln.Addr().(*net.TCPAddr).Port + + done := make(chan struct{}) + mux := http.NewServeMux() + srv := &http.Server{Handler: mux} + + mux.HandleFunc("/"+filepath.Base(path), func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Access-Control-Allow-Origin", "*") + w.Header().Set("Content-Type", "application/octet-stream") + http.ServeFile(w, r, path) + go func() { + close(done) + _ = srv.Shutdown(context.Background()) + }() + }) + + go func() { _ = srv.Serve(ln) }() + + return port, done, nil +} + +// buildOpenURL constructs the hprof-analyzer URL with the ?file= parameter. +// base is the analyzer base URL (trailing slash optional). +func buildOpenURL(base string, port int, filename string) string { + base = strings.TrimRight(base, "/") + return fmt.Sprintf("%s/?file=http://localhost:%d/%s", base, port, filename) +} + +// openBrowser opens url in the system default browser. +// If launch fails, it prints the URL to stdout so the user can open it manually. +func openBrowser(url string) error { + var cmd *exec.Cmd + switch runtime.GOOS { + case "darwin": + cmd = exec.Command("open", url) + case osWindows: + cmd = exec.Command("cmd", "/c", "start", url) + default: + cmd = exec.Command("xdg-open", url) + } + if err := cmd.Start(); err != nil { + fmt.Printf("Opening: %s\n", url) + return nil + } + return nil +} +``` + +- [ ] **Step 4: Run tests to verify they pass** + +```bash +go test -run 'TestServeFileOnce|TestBuildOpenURL' -v ./... 2>&1 +``` + +Expected: all PASS + +- [ ] **Step 5: Commit** + +```bash +git add open.go open_test.go +git commit -m "feat: add serveFileOnce and openBrowser helpers for --open" +``` + +--- + +### Task 2: Flag constants, Options fields, flagDefinitions + +**Files:** +- Modify: `cf_cli_java_plugin.go` + +- [ ] **Step 1: Write the failing test** + +Add to `cf_cli_java_plugin_test.go` (or create it if it doesn't exist — check with `ls *_test.go`): + +```go +func TestParseOptions_Open(t *testing.T) { + p := &JavaPlugin{} + + // --open sets Open=true and OpenURL to default + opts, _, err := p.parseOptions([]string{"heap-dump", "myapp", "--open"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !opts.Open { + t.Error("expected Open=true") + } + if opts.OpenURL != defaultOpenURL { + t.Errorf("expected OpenURL=%q, got %q", defaultOpenURL, opts.OpenURL) + } +} + +func TestParseOptions_OpenURL_ImpliesOpen(t *testing.T) { + p := &JavaPlugin{} + opts, _, err := p.parseOptions([]string{"heap-dump", "myapp", "--open-url", "http://localhost:8080"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !opts.Open { + t.Error("--open-url should imply Open=true") + } + if opts.OpenURL != "http://localhost:8080" { + t.Errorf("unexpected OpenURL: %q", opts.OpenURL) + } +} + +func TestParseOptions_Open_NoDownload_Error(t *testing.T) { + p := &JavaPlugin{} + _, _, err := p.parseOptions([]string{"heap-dump", "myapp", "--open", "--no-download"}) + if err == nil { + t.Fatal("expected error for --open + --no-download, got nil") + } +} +``` + +- [ ] **Step 2: Run tests to verify they fail** + +```bash +go test -run 'TestParseOptions_Open' -v ./... 2>&1 +``` + +Expected: `undefined: defaultOpenURL` or field `Open` not found in `Options` + +- [ ] **Step 3: Add constants, Options fields, flagDefinitions entries** + +In `cf_cli_java_plugin.go`: + +**Add constants** (after `flagCompress = "compress"` on line ~42): + +```go +flagOpen = "open" +flagOpenURL = "open-url" +defaultOpenURL = "https://parttimenerd.github.io/hprof-analyzer" +``` + +**Add fields to `Options` struct** (after `Compress bool` on line ~218): + +```go +Open bool +OpenURL string +``` + +**Add flag definitions** (append after the `flagCompress` entry in `flagDefinitions`, before the closing `}`): + +```go +{ + Name: flagOpen, + Usage: "open the heap dump in the hprof-analyzer web app after downloading", + Type: typeBool, +}, +{ + Name: flagOpenURL, + Usage: "base URL of the hprof-analyzer instance to open (implies --open)", + Type: typeString, +}, +``` + +- [ ] **Step 4: Wire up in `parseOptions`** + +In the `options := &Options{...}` block (after `Compress:` line ~382), add: + +```go +Open: commandFlags.IsSet(flagOpen) || commandFlags.IsSet(flagOpenURL), +OpenURL: func() string { + if u := commandFlags.String(flagOpenURL); u != "" { + return u + } + return defaultOpenURL +}(), +``` + +Add validation after the existing `Redact && RedactComplete` check (after line ~389): + +```go +if options.Open && options.NoDownload { + return nil, nil, &InvalidUsageError{ + message: "Error: flag '--open' requires a local file and cannot be used with '--no-download'", + } +} +``` + +- [ ] **Step 5: Run tests to verify they pass** + +```bash +go test -run 'TestParseOptions_Open' -v ./... 2>&1 +``` + +Expected: all PASS + +- [ ] **Step 6: Verify build is clean** + +```bash +go build ./... 2>&1 +``` + +Expected: no output (success) + +- [ ] **Step 7: Commit** + +```bash +git add cf_cli_java_plugin.go cf_cli_java_plugin_test.go +git commit -m "feat: add --open and --open-url flags to Options and flagDefinitions" +``` + +--- + +### Task 3: Call open logic after file is finalized + +**Files:** +- Modify: `cf_cli_java_plugin.go` (the post-download section, ~lines 1278-1297) + +- [ ] **Step 1: Write the failing test** + +Add to `cf_cli_java_plugin_test.go`: + +```go +func TestParseOptions_Open_DryRun_NoError(t *testing.T) { + // --open + --dry-run is valid (no server started, just prints URL) + p := &JavaPlugin{} + opts, _, err := p.parseOptions([]string{"heap-dump", "myapp", "--open", "--dry-run"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !opts.Open { + t.Error("expected Open=true") + } + if !opts.DryRun { + t.Error("expected DryRun=true") + } +} +``` + +- [ ] **Step 2: Run to verify it passes immediately** (it's a parse-only test, should pass after Task 2) + +```bash +go test -run 'TestParseOptions_Open_DryRun_NoError' -v ./... 2>&1 +``` + +Expected: PASS + +- [ ] **Step 3: Add the open call after file finalization** + +Locate the block in `cf_cli_java_plugin.go` that ends with redaction (around line 1296). The current structure is: + +```go +fmt.Println(utils.ToSentenceCase(command.FileLabel) + " file saved to: " + localFileFullPath) + +if command.Name == cmdHeapDump && (options.Redact || options.RedactComplete) { + // ... redaction sets finalPath ... + fmt.Println("Redacted heap dump saved to: " + finalPath) +} +``` + +Replace that block with: + +```go +fmt.Println(utils.ToSentenceCase(command.FileLabel) + " file saved to: " + localFileFullPath) + +finalLocalPath := localFileFullPath +if command.Name == cmdHeapDump && (options.Redact || options.RedactComplete) { + mode := "lean" + if options.RedactComplete { + mode = "complete" + } + redactBin, rerr := ensureHprofRedact() + if rerr != nil { + return "", fmt.Errorf("hprof-redact unavailable: %w", rerr) + } + localIsGz := strings.HasSuffix(localFileFullPath, ".hprof.gz") + finalPath, rerr := pipeHeapDumpThroughRedact(redactBin, localFileFullPath, mode, options.Compress || localIsGz) + if rerr != nil { + return "", fmt.Errorf("redaction failed (unredacted file at %s): %w", localFileFullPath, rerr) + } + fmt.Println("Redacted heap dump saved to: " + finalPath) + finalLocalPath = finalPath +} + +if command.Name == cmdHeapDump && options.Open { + if options.DryRun { + fmt.Printf("Would open: %s\n", buildOpenURL(options.OpenURL, 0, filepath.Base(finalLocalPath))) + } else { + port, done, serveErr := serveFileOnce(finalLocalPath) + if serveErr != nil { + return "", fmt.Errorf("could not start local file server: %w", serveErr) + } + openURL := buildOpenURL(options.OpenURL, port, filepath.Base(finalLocalPath)) + fmt.Printf("Opening heap dump in browser: %s\n", openURL) + _ = openBrowser(openURL) + <-done + } +} +``` + +Note: the dry-run `PORT` placeholder uses `0` — `buildOpenURL` with port `0` produces `localhost:0` which clearly signals a placeholder. Update `buildOpenURL` in `open.go` to produce `PORT` when port is 0: + +```go +func buildOpenURL(base string, port int, filename string) string { + base = strings.TrimRight(base, "/") + portStr := fmt.Sprintf("%d", port) + if port == 0 { + portStr = "PORT" + } + return fmt.Sprintf("%s/?file=http://localhost:%s/%s", base, portStr, filename) +} +``` + +Also update the test in `open_test.go` for `buildOpenURL` — add a dry-run case: + +```go +{ + "https://parttimenerd.github.io/hprof-analyzer", + 0, + "dump.hprof", + "https://parttimenerd.github.io/hprof-analyzer/?file=http://localhost:PORT/dump.hprof", +}, +``` + +Also add `"path/filepath"` to the import in `cf_cli_java_plugin.go` if not already present. + +- [ ] **Step 4: Verify build is clean** + +```bash +go build ./... 2>&1 +``` + +Expected: no output + +- [ ] **Step 5: Run all tests** + +```bash +go test -v ./... 2>&1 +``` + +Expected: all PASS + +- [ ] **Step 6: Commit** + +```bash +git add cf_cli_java_plugin.go open.go open_test.go +git commit -m "feat: invoke serveFileOnce+openBrowser after heap-dump download for --open" +``` + +--- + +### Task 4: Build, install, and smoke-test against a live CF app + +**Files:** none (verification only) + +- [ ] **Step 1: Build and reinstall the plugin** + +```bash +go build -o build/cf-cli-java-plugin . && cf install-plugin -f build/cf-cli-java-plugin +``` + +Expected: `Plugin java X.X.X successfully installed.` + +- [ ] **Step 2: Dry-run smoke test** + +```bash +cf java heap-dump sapmachine21 --open --dry-run +``` + +Expected output contains: +``` +Would open: https://parttimenerd.github.io/hprof-analyzer/?file=http://localhost:PORT/sapmachine21-heapdump-.hprof +``` + +- [ ] **Step 3: Dry-run with custom URL** + +```bash +cf java heap-dump sapmachine21 --open-url http://localhost:8080 --dry-run +``` + +Expected: URL starts with `http://localhost:8080/?file=http://localhost:PORT/` + +- [ ] **Step 4: Dry-run `--open` + `--no-download` errors** + +```bash +cf java heap-dump sapmachine21 --open --no-download --dry-run 2>&1; echo "EXIT: $?" +``` + +Expected: error message `'--open' requires a local file and cannot be used with '--no-download'`, exit 1 + +- [ ] **Step 5: Dry-run `--open` + `--compress`** + +```bash +cf java heap-dump sapmachine21 --open --compress --dry-run +``` + +Expected: URL path ends with `.hprof.gz` + +- [ ] **Step 6: Dry-run `--open` + `--redact`** + +```bash +cf java heap-dump sapmachine21 --open --redact --dry-run +``` + +Expected: URL path ends with `-redacted.hprof` + +- [ ] **Step 7: Commit smoke-test confirmation (no code change needed)** + +If all above pass with no code changes required: +```bash +git commit --allow-empty -m "test: smoke-tested --open flag against live CF app" +``` + +--- + +## Self-Review Notes + +- **Spec coverage:** All requirements covered — `--open`, `--open-url`, `--open-url` implies `--open`, `--open` + `--no-download` error, compatible with `--redact`/`--compress`, dry-run prints placeholder URL, serve-once lifecycle, cross-platform `openBrowser`. +- **Placeholder scan:** All code blocks are complete and runnable. +- **Type consistency:** `serveFileOnce` returns `(int, <-chan struct{}, error)` in Task 1 and is called identically in Task 3. `buildOpenURL(base string, port int, filename string) string` is defined in Task 1 and used the same way in Task 3. +- **`filepath` import:** Task 3 notes to add `"path/filepath"` — check if already imported before adding. +- **`finalLocalPath` variable:** Introduced in Task 3 to track the post-redact path; the existing `localFileFullPath` variable is preserved unchanged for the error-path retry message at line ~1303. From 8c3b131ec1b94aab151099991050ecbf96566c55 Mon Sep 17 00:00:00 2001 From: Johannes Bechberger Date: Thu, 17 Sep 2026 13:55:05 +0200 Subject: [PATCH 07/39] feat: add serveFileOnce and openBrowser helpers for --open --- open.go | 88 ++++++++++++++++++++++++++++++++++++++++++ open_test.go | 106 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 194 insertions(+) create mode 100644 open.go create mode 100644 open_test.go diff --git a/open.go b/open.go new file mode 100644 index 0000000..0251fc0 --- /dev/null +++ b/open.go @@ -0,0 +1,88 @@ +/* + * Copyright (c) 2024 SAP SE or an SAP affiliate company. All rights reserved. + * This file is licensed under the Apache Software License, v. 2 except as noted + * otherwise in the LICENSE file at the root of the repository. + */ + +package main + +import ( + "fmt" + "net" + "net/http" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "time" +) + +// serveFileOnce starts a local HTTP server on a random port that serves path +// exactly once. It returns the bound port and a channel that is closed when +// the first GET request completes. The server shuts itself down after serving. +// Returns an error if the file does not exist or the port cannot be bound. +func serveFileOnce(path string) (int, <-chan struct{}, error) { + if _, err := os.Stat(path); err != nil { + return 0, nil, fmt.Errorf("file not found: %w", err) + } + + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + return 0, nil, fmt.Errorf("could not bind local port: %w", err) + } + port := ln.Addr().(*net.TCPAddr).Port + + done := make(chan struct{}) + mux := http.NewServeMux() + srv := &http.Server{ + Handler: mux, + ReadHeaderTimeout: 10 * time.Second, + } + + mux.HandleFunc("/"+filepath.Base(path), func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Access-Control-Allow-Origin", "*") + w.Header().Set("Content-Type", "application/octet-stream") + http.ServeFile(w, r, path) + ctx := r.Context() + go func() { + close(done) + _ = srv.Shutdown(ctx) + }() + }) + + go func() { _ = srv.Serve(ln) }() + + return port, done, nil +} + +// buildOpenURL constructs the hprof-analyzer URL with the ?file= parameter. +// base is the analyzer base URL (trailing slash optional). +// Use port=0 to produce a PORT placeholder (for dry-run output). +func buildOpenURL(base string, port int, filename string) string { + base = strings.TrimRight(base, "/") + portStr := fmt.Sprintf("%d", port) + if port == 0 { + portStr = "PORT" + } + return fmt.Sprintf("%s/?file=http://localhost:%s/%s", base, portStr, filename) +} + +// openBrowser opens url in the system default browser. +// If launch fails, prints the URL to stdout so the user can open it manually. +// +//nolint:unused +func openBrowser(url string) { + var cmd *exec.Cmd + switch runtime.GOOS { + case "darwin": + cmd = exec.Command("open", url) + case osWindows: + cmd = exec.Command("cmd", "/c", "start", url) + default: + cmd = exec.Command("xdg-open", url) + } + if err := cmd.Start(); err != nil { + fmt.Printf("Opening: %s\n", url) + } +} diff --git a/open_test.go b/open_test.go new file mode 100644 index 0000000..b535d19 --- /dev/null +++ b/open_test.go @@ -0,0 +1,106 @@ +package main + +import ( + "fmt" + "io" + "net/http" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestServeFileOnce(t *testing.T) { + tmp := t.TempDir() + p := filepath.Join(tmp, "test.hprof") + if err := os.WriteFile(p, []byte("HEAP_CONTENT"), 0o600); err != nil { + t.Fatal(err) + } + + port, done, err := serveFileOnce(p) + if err != nil { + t.Fatalf("serveFileOnce: %v", err) + } + if port <= 0 { + t.Fatalf("expected positive port, got %d", port) + } + + url := fmt.Sprintf("http://localhost:%d/test.hprof", port) + resp, err := http.Get(url) //nolint:noctx,gosec + if err != nil { + t.Fatalf("GET %s: %v", url, err) + } + defer func() { + if closeErr := resp.Body.Close(); closeErr != nil { + t.Errorf("body close: %v", closeErr) + } + }() + + if resp.StatusCode != http.StatusOK { + t.Fatalf("expected 200, got %d", resp.StatusCode) + } + if ct := resp.Header.Get("Content-Type"); ct != "application/octet-stream" { + t.Errorf("Content-Type: want application/octet-stream, got %s", ct) + } + if acao := resp.Header.Get("Access-Control-Allow-Origin"); acao != "*" { + t.Errorf("Access-Control-Allow-Origin: want *, got %s", acao) + } + body, _ := io.ReadAll(resp.Body) + if string(body) != "HEAP_CONTENT" { + t.Errorf("body: want HEAP_CONTENT, got %s", body) + } + + select { + case <-done: + default: + t.Error("done channel not closed after successful GET") + } +} + +func TestServeFileOnce_MissingFile(t *testing.T) { + _, _, err := serveFileOnce("/nonexistent/path/dump.hprof") + if err == nil { + t.Fatal("expected error for missing file, got nil") + } +} + +func TestBuildOpenURL(t *testing.T) { + cases := []struct { + base string + port int + filename string + want string + }{ + { + "https://parttimenerd.github.io/hprof-analyzer", + 54321, + "myapp-heapdump-abc.hprof", + "https://parttimenerd.github.io/hprof-analyzer/?file=http://localhost:54321/myapp-heapdump-abc.hprof", + }, + { + "https://parttimenerd.github.io/hprof-analyzer/", + 9000, + "dump.hprof.gz", + "https://parttimenerd.github.io/hprof-analyzer/?file=http://localhost:9000/dump.hprof.gz", + }, + { + "https://parttimenerd.github.io/hprof-analyzer", + 0, + "dump.hprof", + "https://parttimenerd.github.io/hprof-analyzer/?file=http://localhost:PORT/dump.hprof", + }, + } + for _, tc := range cases { + got := buildOpenURL(tc.base, tc.port, tc.filename) + if got != tc.want { + t.Errorf("buildOpenURL(%q, %d, %q)\n want %q\n got %q", tc.base, tc.port, tc.filename, tc.want, got) + } + } +} + +func TestBuildOpenURL_TrailingSlash(t *testing.T) { + url := buildOpenURL("https://example.com/analyzer/", 1234, "dump.hprof") + if strings.Contains(url, "//?") { + t.Errorf("double slash before ?: %s", url) + } +} From 19ea35428e4c913ed214fed35fa7a46c9359e7cd Mon Sep 17 00:00:00 2001 From: Johannes Bechberger Date: Thu, 17 Sep 2026 13:58:21 +0200 Subject: [PATCH 08/39] fix: address code quality issues in open.go - Use context.WithTimeout(context.Background(), 5s) for srv.Shutdown instead of the request context (which may already be canceled); suppress contextcheck lint with explanation since this is intentional - Add empty title arg to Windows `cmd /c start` to avoid URL being interpreted as the window title - Replace non-blocking done-channel check in test with a 2s timeout select to avoid a false-pass race condition --- open.go | 12 ++++++++---- open_test.go | 3 ++- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/open.go b/open.go index 0251fc0..b5c5811 100644 --- a/open.go +++ b/open.go @@ -7,6 +7,7 @@ package main import ( + "context" "fmt" "net" "net/http" @@ -44,11 +45,14 @@ func serveFileOnce(path string) (int, <-chan struct{}, error) { w.Header().Set("Access-Control-Allow-Origin", "*") w.Header().Set("Content-Type", "application/octet-stream") http.ServeFile(w, r, path) - ctx := r.Context() - go func() { + // Intentionally not using r.Context(): the request context is canceled as + // soon as the handler returns, but Shutdown must outlive the request. + shutdownCtx, shutdownCancel := context.WithTimeout(context.Background(), 5*time.Second) //nolint:contextcheck + go func(ctx context.Context, cancel context.CancelFunc) { //nolint:contextcheck + defer cancel() close(done) _ = srv.Shutdown(ctx) - }() + }(shutdownCtx, shutdownCancel) }) go func() { _ = srv.Serve(ln) }() @@ -78,7 +82,7 @@ func openBrowser(url string) { case "darwin": cmd = exec.Command("open", url) case osWindows: - cmd = exec.Command("cmd", "/c", "start", url) + cmd = exec.Command("cmd", "/c", "start", "", url) default: cmd = exec.Command("xdg-open", url) } diff --git a/open_test.go b/open_test.go index b535d19..7e27a4e 100644 --- a/open_test.go +++ b/open_test.go @@ -8,6 +8,7 @@ import ( "path/filepath" "strings" "testing" + "time" ) func TestServeFileOnce(t *testing.T) { @@ -52,7 +53,7 @@ func TestServeFileOnce(t *testing.T) { select { case <-done: - default: + case <-time.After(2 * time.Second): t.Error("done channel not closed after successful GET") } } From 82d626d0d69eac1ead84af70ea80280a449e07c4 Mon Sep 17 00:00:00 2001 From: Johannes Bechberger Date: Thu, 17 Sep 2026 14:01:03 +0200 Subject: [PATCH 09/39] feat: add --open and --open-url flags to Options and flagDefinitions --- cf_cli_java_plugin.go | 28 +++++++++++++++++ cf_cli_java_plugin_test.go | 63 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+) create mode 100644 cf_cli_java_plugin_test.go diff --git a/cf_cli_java_plugin.go b/cf_cli_java_plugin.go index ff38a4a..f5afd63 100644 --- a/cf_cli_java_plugin.go +++ b/cf_cli_java_plugin.go @@ -40,6 +40,9 @@ const ( flagRedact = "redact" flagRedactComplete = "redact-complete" flagCompress = "compress" + flagOpen = "open" + flagOpenURL = "open-url" + defaultOpenURL = "https://parttimenerd.github.io/hprof-analyzer" osWindows = "windows" cmdHeapDump = "heap-dump" typeBool = "bool" @@ -216,6 +219,8 @@ type Options struct { Redact bool RedactComplete bool Compress bool + Open bool + OpenURL string } // FlagDefinition holds metadata for a command-line flag @@ -309,6 +314,16 @@ var flagDefinitions = []FlagDefinition{ Usage: "compress heap dump on container using jmap gz=1 (JDK 17+) to reduce transfer size; output is .hprof.gz", Type: typeBool, }, + { + Name: flagOpen, + Usage: "open the heap dump in the hprof-analyzer web app after downloading", + Type: typeBool, + }, + { + Name: flagOpenURL, + Usage: "base URL of the hprof-analyzer instance to open (implies --open)", + Type: typeString, + }, } func (c *JavaPlugin) createOptionsParser() flags.FlagContext { @@ -380,6 +395,13 @@ func (c *JavaPlugin) parseOptions(args []string) (*Options, []string, error) { Redact: commandFlags.IsSet(flagRedact), RedactComplete: commandFlags.IsSet(flagRedactComplete), Compress: commandFlags.IsSet(flagCompress), + Open: commandFlags.IsSet(flagOpen) || commandFlags.IsSet(flagOpenURL), + OpenURL: func() string { + if u := commandFlags.String(flagOpenURL); u != "" { + return u + } + return defaultOpenURL + }(), } if options.Redact && options.RedactComplete { @@ -388,6 +410,12 @@ func (c *JavaPlugin) parseOptions(args []string) (*Options, []string, error) { } } + if options.Open && options.NoDownload { + return nil, nil, &InvalidUsageError{ + message: "Error: flag '--open' requires a local file and cannot be used with '--no-download'", + } + } + return options, commandFlags.Args(), nil } diff --git a/cf_cli_java_plugin_test.go b/cf_cli_java_plugin_test.go new file mode 100644 index 0000000..7927416 --- /dev/null +++ b/cf_cli_java_plugin_test.go @@ -0,0 +1,63 @@ +/* + * Copyright (c) 2024 SAP SE or an SAP affiliate company. All rights reserved. + * This file is licensed under the Apache Software License, v. 2 except as noted + * otherwise in the LICENSE file at the root of the repository. + */ + +package main + +import ( + "testing" +) + +const testAppName = "myapp" + +func TestParseOptions_Open(t *testing.T) { + p := &JavaPlugin{} + opts, _, err := p.parseOptions([]string{cmdHeapDump, testAppName, "--" + flagOpen}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !opts.Open { + t.Error("expected Open=true") + } + if opts.OpenURL != defaultOpenURL { + t.Errorf("expected OpenURL=%q, got %q", defaultOpenURL, opts.OpenURL) + } +} + +func TestParseOptions_OpenURL_ImpliesOpen(t *testing.T) { + p := &JavaPlugin{} + opts, _, err := p.parseOptions([]string{cmdHeapDump, testAppName, "--" + flagOpenURL, "http://localhost:8080"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !opts.Open { + t.Error("--open-url should imply Open=true") + } + if opts.OpenURL != "http://localhost:8080" { + t.Errorf("unexpected OpenURL: %q", opts.OpenURL) + } +} + +func TestParseOptions_Open_NoDownload_Error(t *testing.T) { + p := &JavaPlugin{} + _, _, err := p.parseOptions([]string{cmdHeapDump, testAppName, "--" + flagOpen, "--" + flagNoDownload}) + if err == nil { + t.Fatal("expected error for --open + --no-download, got nil") + } +} + +func TestParseOptions_Open_DryRun_NoError(t *testing.T) { + p := &JavaPlugin{} + opts, _, err := p.parseOptions([]string{cmdHeapDump, testAppName, "--" + flagOpen, "--dry-run"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !opts.Open { + t.Error("expected Open=true") + } + if !opts.DryRun { + t.Error("expected DryRun=true") + } +} From 101d6583313813ae4f5bdb86bfaf9b4e71dbc6bd Mon Sep 17 00:00:00 2001 From: Johannes Bechberger Date: Thu, 17 Sep 2026 14:03:04 +0200 Subject: [PATCH 10/39] feat: invoke serveFileOnce+openBrowser after heap-dump download for --open --- cf_cli_java_plugin.go | 17 +++++++++++++++++ open.go | 2 -- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/cf_cli_java_plugin.go b/cf_cli_java_plugin.go index f5afd63..b15d4b9 100644 --- a/cf_cli_java_plugin.go +++ b/cf_cli_java_plugin.go @@ -1307,6 +1307,7 @@ func (c *JavaPlugin) execute(_ plugin.CliConnection, args []string) (string, err c.logVerbosef("File download completed successfully") fmt.Println(utils.ToSentenceCase(command.FileLabel) + " file saved to: " + localFileFullPath) + finalLocalPath := localFileFullPath if command.Name == cmdHeapDump && (options.Redact || options.RedactComplete) { mode := "lean" if options.RedactComplete { @@ -1322,6 +1323,22 @@ func (c *JavaPlugin) execute(_ plugin.CliConnection, args []string) (string, err return "", fmt.Errorf("redaction failed (unredacted file at %s): %w", localFileFullPath, rerr) } fmt.Println("Redacted heap dump saved to: " + finalPath) + finalLocalPath = finalPath + } + + if command.Name == cmdHeapDump && options.Open { + if options.DryRun { + fmt.Printf("Would open: %s\n", buildOpenURL(options.OpenURL, 0, filepath.Base(finalLocalPath))) + } else { + port, done, serveErr := serveFileOnce(finalLocalPath) + if serveErr != nil { + return "", fmt.Errorf("could not start local file server: %w", serveErr) + } + openURL := buildOpenURL(options.OpenURL, port, filepath.Base(finalLocalPath)) + fmt.Printf("Opening heap dump in browser: %s\n", openURL) + openBrowser(openURL) + <-done + } } } else { c.logVerbosef("File download failed: %v", err) diff --git a/open.go b/open.go index b5c5811..198da85 100644 --- a/open.go +++ b/open.go @@ -74,8 +74,6 @@ func buildOpenURL(base string, port int, filename string) string { // openBrowser opens url in the system default browser. // If launch fails, prints the URL to stdout so the user can open it manually. -// -//nolint:unused func openBrowser(url string) { var cmd *exec.Cmd switch runtime.GOOS { From 964e2eaf623d1fc7eef7474c2fc66661187cd722 Mon Sep 17 00:00:00 2001 From: Johannes Bechberger Date: Thu, 17 Sep 2026 14:26:23 +0200 Subject: [PATCH 11/39] fix: stream file directly via io.Copy to prevent http.ServeFile path traversal --- open.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/open.go b/open.go index 198da85..5c1cbc1 100644 --- a/open.go +++ b/open.go @@ -9,6 +9,7 @@ package main import ( "context" "fmt" + "io" "net" "net/http" "os" @@ -42,9 +43,18 @@ func serveFileOnce(path string) (int, <-chan struct{}, error) { } mux.HandleFunc("/"+filepath.Base(path), func(w http.ResponseWriter, r *http.Request) { + // Open the specific file directly rather than using http.ServeFile, which + // follows path cleaning and redirects and could expose other files. + f, ferr := os.Open(path) //nolint:gosec // path comes from plugin internals, not user input + if ferr != nil { + http.Error(w, "file unavailable", http.StatusInternalServerError) + return + } + defer func() { _ = f.Close() }() w.Header().Set("Access-Control-Allow-Origin", "*") w.Header().Set("Content-Type", "application/octet-stream") - http.ServeFile(w, r, path) + w.WriteHeader(http.StatusOK) + _, _ = io.Copy(w, f) // Intentionally not using r.Context(): the request context is canceled as // soon as the handler returns, but Shutdown must outlive the request. shutdownCtx, shutdownCancel := context.WithTimeout(context.Background(), 5*time.Second) //nolint:contextcheck From a79a3daabdcee351f6919ad0ebbb990aa2744cc6 Mon Sep 17 00:00:00 2001 From: Johannes Bechberger Date: Thu, 17 Sep 2026 14:33:14 +0200 Subject: [PATCH 12/39] fix: use random token URL path in serveFileOnce to prevent filename leakage and restrict access to single file --- cf_cli_java_plugin.go | 13 +++++----- open.go | 47 +++++++++++++++++++++++------------- open_test.go | 56 ++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 90 insertions(+), 26 deletions(-) diff --git a/cf_cli_java_plugin.go b/cf_cli_java_plugin.go index b15d4b9..88aeb52 100644 --- a/cf_cli_java_plugin.go +++ b/cf_cli_java_plugin.go @@ -53,6 +53,7 @@ const ( labelJFR = "JFR recording" partJFR = "jfr" extHprof = ".hprof" + extHprofGz = ".hprof.gz" ) // JavaPlugin is a CF CLI plugin that supports taking heap and thread dumps on demand @@ -1248,7 +1249,7 @@ func (c *JavaPlugin) execute(_ plugin.CliConnection, args []string) (string, err case extHprof: c.logVerbosef("Finding heap dump file") finalFile, err = utils.FindHeapDumpFile(cfSSHArguments, fileName, fspath, applicationName+"-"+command.FileNamePart) - case ".hprof.gz": + case extHprofGz: c.logVerbosef("Finding compressed heap dump file") finalFile, err = utils.FindHeapDumpGzFile(cfSSHArguments, fileName, fspath, applicationName+"-"+command.FileNamePart) case ".jfr": @@ -1285,7 +1286,7 @@ func (c *JavaPlugin) execute(_ plugin.CliConnection, args []string) (string, err switch { case remoteIsGz && options.Compress: // User asked for .hprof.gz locally → keep compressed - localFileExt = ".hprof.gz" + localFileExt = extHprofGz case !remoteIsGz && options.Compress: fmt.Fprintf(os.Stderr, "Warning: remote jmap does not support gz compression (JDK 17+ required); downloading uncompressed\n") case remoteIsGz: @@ -1317,7 +1318,7 @@ func (c *JavaPlugin) execute(_ plugin.CliConnection, args []string) (string, err if rerr != nil { return "", fmt.Errorf("hprof-redact unavailable: %w", rerr) } - localIsGz := strings.HasSuffix(localFileFullPath, ".hprof.gz") + localIsGz := strings.HasSuffix(localFileFullPath, extHprofGz) finalPath, rerr := pipeHeapDumpThroughRedact(redactBin, localFileFullPath, mode, options.Compress || localIsGz) if rerr != nil { return "", fmt.Errorf("redaction failed (unredacted file at %s): %w", localFileFullPath, rerr) @@ -1328,13 +1329,13 @@ func (c *JavaPlugin) execute(_ plugin.CliConnection, args []string) (string, err if command.Name == cmdHeapDump && options.Open { if options.DryRun { - fmt.Printf("Would open: %s\n", buildOpenURL(options.OpenURL, 0, filepath.Base(finalLocalPath))) + fmt.Printf("Would open: %s\n", buildOpenURL(options.OpenURL, 0, "TOKEN.hprof")) } else { - port, done, serveErr := serveFileOnce(finalLocalPath) + port, urlFile, done, serveErr := serveFileOnce(finalLocalPath) if serveErr != nil { return "", fmt.Errorf("could not start local file server: %w", serveErr) } - openURL := buildOpenURL(options.OpenURL, port, filepath.Base(finalLocalPath)) + openURL := buildOpenURL(options.OpenURL, port, urlFile) fmt.Printf("Opening heap dump in browser: %s\n", openURL) openBrowser(openURL) <-done diff --git a/open.go b/open.go index 5c1cbc1..d084519 100644 --- a/open.go +++ b/open.go @@ -8,43 +8,56 @@ package main import ( "context" + "crypto/rand" + "encoding/hex" "fmt" "io" "net" "net/http" "os" "os/exec" - "path/filepath" "runtime" "strings" "time" ) // serveFileOnce starts a local HTTP server on a random port that serves path -// exactly once. It returns the bound port and a channel that is closed when -// the first GET request completes. The server shuts itself down after serving. -// Returns an error if the file does not exist or the port cannot be bound. -func serveFileOnce(path string) (int, <-chan struct{}, error) { - if _, err := os.Stat(path); err != nil { - return 0, nil, fmt.Errorf("file not found: %w", err) +// exactly once. The file is exposed under a random token path (e.g. /a3f9c2.hprof) +// so the local filename is never leaked and only the holder of the URL can fetch it. +// Returns the bound port, the randomised URL path segment, and a channel that +// closes when the first GET request completes. +func serveFileOnce(path string) (port int, urlFile string, done <-chan struct{}, err error) { + if _, err = os.Stat(path); err != nil { + return 0, "", nil, fmt.Errorf("file not found: %w", err) } + // Build a random token + preserve only the file extension (.hprof or .hprof.gz). + var tokenBytes [8]byte + if _, err = rand.Read(tokenBytes[:]); err != nil { + return 0, "", nil, fmt.Errorf("could not generate token: %w", err) + } + token := hex.EncodeToString(tokenBytes[:]) + ext := extHprof + if strings.HasSuffix(path, extHprofGz) { + ext = extHprofGz + } + urlFile = token + ext + ln, err := net.Listen("tcp", "127.0.0.1:0") if err != nil { - return 0, nil, fmt.Errorf("could not bind local port: %w", err) + return 0, "", nil, fmt.Errorf("could not bind local port: %w", err) } - port := ln.Addr().(*net.TCPAddr).Port + port = ln.Addr().(*net.TCPAddr).Port - done := make(chan struct{}) + doneCh := make(chan struct{}) mux := http.NewServeMux() srv := &http.Server{ Handler: mux, ReadHeaderTimeout: 10 * time.Second, } - mux.HandleFunc("/"+filepath.Base(path), func(w http.ResponseWriter, r *http.Request) { - // Open the specific file directly rather than using http.ServeFile, which - // follows path cleaning and redirects and could expose other files. + // Register only the exact random path — any other request gets 404. + mux.HandleFunc("/"+urlFile, func(w http.ResponseWriter, r *http.Request) { f, ferr := os.Open(path) //nolint:gosec // path comes from plugin internals, not user input if ferr != nil { http.Error(w, "file unavailable", http.StatusInternalServerError) @@ -55,19 +68,19 @@ func serveFileOnce(path string) (int, <-chan struct{}, error) { w.Header().Set("Content-Type", "application/octet-stream") w.WriteHeader(http.StatusOK) _, _ = io.Copy(w, f) - // Intentionally not using r.Context(): the request context is canceled as - // soon as the handler returns, but Shutdown must outlive the request. + // Use a fresh context: r.Context() is canceled when the handler returns, + // but Shutdown must outlive the request. shutdownCtx, shutdownCancel := context.WithTimeout(context.Background(), 5*time.Second) //nolint:contextcheck go func(ctx context.Context, cancel context.CancelFunc) { //nolint:contextcheck defer cancel() - close(done) + close(doneCh) _ = srv.Shutdown(ctx) }(shutdownCtx, shutdownCancel) }) go func() { _ = srv.Serve(ln) }() - return port, done, nil + return port, urlFile, doneCh, nil } // buildOpenURL constructs the hprof-analyzer URL with the ?file= parameter. diff --git a/open_test.go b/open_test.go index 7e27a4e..b5f5be8 100644 --- a/open_test.go +++ b/open_test.go @@ -18,7 +18,7 @@ func TestServeFileOnce(t *testing.T) { t.Fatal(err) } - port, done, err := serveFileOnce(p) + port, urlFile, done, err := serveFileOnce(p) if err != nil { t.Fatalf("serveFileOnce: %v", err) } @@ -26,7 +26,15 @@ func TestServeFileOnce(t *testing.T) { t.Fatalf("expected positive port, got %d", port) } - url := fmt.Sprintf("http://localhost:%d/test.hprof", port) + // urlFile must have .hprof extension and contain only the token (no path separators) + if !strings.HasSuffix(urlFile, ".hprof") { + t.Errorf("urlFile %q does not end with .hprof", urlFile) + } + if strings.ContainsAny(urlFile, "/\\") { + t.Errorf("urlFile %q contains path separators", urlFile) + } + + url := fmt.Sprintf("http://localhost:%d/%s", port, urlFile) resp, err := http.Get(url) //nolint:noctx,gosec if err != nil { t.Fatalf("GET %s: %v", url, err) @@ -58,8 +66,50 @@ func TestServeFileOnce(t *testing.T) { } } +func TestServeFileOnce_WrongPath404(t *testing.T) { + tmp := t.TempDir() + p := filepath.Join(tmp, "test.hprof") + if err := os.WriteFile(p, []byte("SECRET"), 0o600); err != nil { + t.Fatal(err) + } + + port, _, _, err := serveFileOnce(p) + if err != nil { + t.Fatalf("serveFileOnce: %v", err) + } + + // Any path other than the exact random token must return 404 + for _, badPath := range []string{"/test.hprof", "/", "/other.hprof", "/../../etc/passwd"} { + url := fmt.Sprintf("http://localhost:%d%s", port, badPath) + resp, gerr := http.Get(url) //nolint:noctx,gosec + if gerr != nil { + continue // server may have shut down already, that's fine + } + _ = resp.Body.Close() + if resp.StatusCode == http.StatusOK { + t.Errorf("GET %s: expected non-200, got 200", url) + } + } +} + +func TestServeFileOnce_GzExtension(t *testing.T) { + tmp := t.TempDir() + p := filepath.Join(tmp, "test.hprof.gz") + if err := os.WriteFile(p, []byte("GZ"), 0o600); err != nil { + t.Fatal(err) + } + + _, urlFile, _, err := serveFileOnce(p) + if err != nil { + t.Fatalf("serveFileOnce: %v", err) + } + if !strings.HasSuffix(urlFile, ".hprof.gz") { + t.Errorf("urlFile %q does not end with .hprof.gz", urlFile) + } +} + func TestServeFileOnce_MissingFile(t *testing.T) { - _, _, err := serveFileOnce("/nonexistent/path/dump.hprof") + _, _, _, err := serveFileOnce("/nonexistent/path/dump.hprof") if err == nil { t.Fatal("expected error for missing file, got nil") } From 43d22607f4332cb2f9e2ae0dad2355bb3be79cf8 Mon Sep 17 00:00:00 2001 From: Johannes Bechberger Date: Thu, 17 Sep 2026 14:35:07 +0200 Subject: [PATCH 13/39] fix: print Would-open line before dry-run SSH command output for --open --dry-run --- cf_cli_java_plugin.go | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/cf_cli_java_plugin.go b/cf_cli_java_plugin.go index 88aeb52..e41dd86 100644 --- a/cf_cli_java_plugin.go +++ b/cf_cli_java_plugin.go @@ -1218,6 +1218,13 @@ func (c *JavaPlugin) execute(_ plugin.CliConnection, args []string) (string, err // to prevent the shell processing it from running it in local escapedCommand := strings.ReplaceAll(remoteCommand, "'", "'\\''") cfSSHArguments = append(cfSSHArguments, "'"+escapedCommand+"'") + if command.Name == cmdHeapDump && options.Open { + ext := extHprof + if options.Compress { + ext = extHprofGz + } + fmt.Printf("Would open: %s\n", buildOpenURL(options.OpenURL, 0, "TOKEN"+ext)) + } return "cf " + strings.Join(cfSSHArguments, " "), nil } @@ -1328,18 +1335,14 @@ func (c *JavaPlugin) execute(_ plugin.CliConnection, args []string) (string, err } if command.Name == cmdHeapDump && options.Open { - if options.DryRun { - fmt.Printf("Would open: %s\n", buildOpenURL(options.OpenURL, 0, "TOKEN.hprof")) - } else { - port, urlFile, done, serveErr := serveFileOnce(finalLocalPath) - if serveErr != nil { - return "", fmt.Errorf("could not start local file server: %w", serveErr) - } - openURL := buildOpenURL(options.OpenURL, port, urlFile) - fmt.Printf("Opening heap dump in browser: %s\n", openURL) - openBrowser(openURL) - <-done + port, urlFile, done, serveErr := serveFileOnce(finalLocalPath) + if serveErr != nil { + return "", fmt.Errorf("could not start local file server: %w", serveErr) } + openURL := buildOpenURL(options.OpenURL, port, urlFile) + fmt.Printf("Opening heap dump in browser: %s\n", openURL) + openBrowser(openURL) + <-done } } else { c.logVerbosef("File download failed: %v", err) From 22fc173c563c2349f27646a11435802dd10f0cb0 Mon Sep 17 00:00:00 2001 From: Johannes Bechberger Date: Thu, 17 Sep 2026 14:39:01 +0200 Subject: [PATCH 14/39] fix: handle CORS OPTIONS preflight without triggering server shutdown in serveFileOnce --- open.go | 12 +++++++++++- open_test.go | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/open.go b/open.go index d084519..fd7ff40 100644 --- a/open.go +++ b/open.go @@ -58,13 +58,23 @@ func serveFileOnce(path string) (port int, urlFile string, done <-chan struct{}, // Register only the exact random path — any other request gets 404. mux.HandleFunc("/"+urlFile, func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Access-Control-Allow-Origin", "*") + // Answer CORS preflight without serving the file or triggering shutdown. + if r.Method == http.MethodOptions { + w.Header().Set("Access-Control-Allow-Methods", "GET, OPTIONS") + w.WriteHeader(http.StatusNoContent) + return + } + if r.Method != http.MethodGet { + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + return + } f, ferr := os.Open(path) //nolint:gosec // path comes from plugin internals, not user input if ferr != nil { http.Error(w, "file unavailable", http.StatusInternalServerError) return } defer func() { _ = f.Close() }() - w.Header().Set("Access-Control-Allow-Origin", "*") w.Header().Set("Content-Type", "application/octet-stream") w.WriteHeader(http.StatusOK) _, _ = io.Copy(w, f) diff --git a/open_test.go b/open_test.go index b5f5be8..c3322e9 100644 --- a/open_test.go +++ b/open_test.go @@ -115,6 +115,52 @@ func TestServeFileOnce_MissingFile(t *testing.T) { } } +func TestServeFileOnce_OptionsPreflight(t *testing.T) { + tmp := t.TempDir() + p := filepath.Join(tmp, "test.hprof") + if err := os.WriteFile(p, []byte("DATA"), 0o600); err != nil { + t.Fatal(err) + } + + port, urlFile, done, err := serveFileOnce(p) + if err != nil { + t.Fatalf("serveFileOnce: %v", err) + } + + url := fmt.Sprintf("http://localhost:%d/%s", port, urlFile) + + // OPTIONS preflight must not trigger shutdown + req, _ := http.NewRequest(http.MethodOptions, url, nil) //nolint:noctx + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("OPTIONS %s: %v", url, err) + } + _ = resp.Body.Close() + if resp.StatusCode != http.StatusNoContent { + t.Errorf("OPTIONS: want 204, got %d", resp.StatusCode) + } + select { + case <-done: + t.Error("done channel closed after OPTIONS — server shut down prematurely") + default: + } + + // Subsequent GET must still succeed and close done + resp2, err := http.Get(url) //nolint:noctx,gosec + if err != nil { + t.Fatalf("GET after OPTIONS: %v", err) + } + defer func() { _ = resp2.Body.Close() }() + if resp2.StatusCode != http.StatusOK { + t.Errorf("GET after OPTIONS: want 200, got %d", resp2.StatusCode) + } + select { + case <-done: + case <-time.After(2 * time.Second): + t.Error("done channel not closed after GET") + } +} + func TestBuildOpenURL(t *testing.T) { cases := []struct { base string From 91d8a55fa0b5254a9c3c2d44a27c71c50b2c24e7 Mon Sep 17 00:00:00 2001 From: Johannes Bechberger Date: Thu, 17 Sep 2026 14:44:06 +0200 Subject: [PATCH 15/39] docs: document --open and --open-url flags in README and CHANGELOG --- CHANGELOG.md | 6 ++++++ README.md | 10 ++++++++++ 2 files changed, 16 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c2a87a0..3d05a6c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), on the container). Prints a warning and falls back to uncompressed on older JDKs. - Transparent compressed transfer: on JDK 17+ containers, the plugin automatically uses `jmap gz=1` to reduce transfer size even without `--compress`, decompressing on the fly so the local file is always a plain `.hprof`. +- `heap-dump --open`: after downloading (and optionally redacting/compressing) the dump, spins up a temporary local + HTTP server and opens the [hprof-analyzer](https://parttimenerd.github.io/hprof-analyzer) web app in the default + browser with the dump pre-loaded. The server serves the file exactly once via a random token URL and shuts down + automatically after the browser fetches it. +- `heap-dump --open-url `: override the hprof-analyzer base URL (e.g. a locally running instance). Implies + `--open`. ### Changed diff --git a/README.md b/README.md index fdac700..f640b79 100644 --- a/README.md +++ b/README.md @@ -17,6 +17,7 @@ Currently, it allows you to: dependency graphs, and more): bundled directly in the plugin, requires Java 17+ locally - Redact heap dumps before saving to remove sensitive data (`--redact`, `--redact-complete`) - Reduce transfer size by compressing heap dumps over SSH (`--compress`) +- Open heap dumps directly in the [hprof-analyzer](https://parttimenerd.github.io/hprof-analyzer) web app after downloading (`--open`) ## Installation @@ -141,6 +142,15 @@ cf java heap-dump $APP_NAME --compress # saves as .hprof.gz # Redact and compress cf java heap-dump $APP_NAME --redact --compress + +# Open in hprof-analyzer web app after downloading (spins up a local server, opens browser) +cf java heap-dump $APP_NAME --open + +# Open with redaction and compression applied first +cf java heap-dump $APP_NAME --open --redact --compress + +# Open using a locally running hprof-analyzer instance +cf java heap-dump $APP_NAME --open-url http://localhost:8080 ``` Getting a thread dump: From b46bb1f6b706ecdbcb5524515a4be3f4fcf29fc8 Mon Sep 17 00:00:00 2001 From: Johannes Bechberger Date: Thu, 17 Sep 2026 15:12:03 +0200 Subject: [PATCH 16/39] docs: document macOS firewall permission dialog for --open flag --- README.md | 5 +++++ .../specs/2026-09-17-open-heap-dump-design.md | 12 ++++++++++++ 2 files changed, 17 insertions(+) diff --git a/README.md b/README.md index f640b79..2c2d17e 100644 --- a/README.md +++ b/README.md @@ -153,6 +153,11 @@ cf java heap-dump $APP_NAME --open --redact --compress cf java heap-dump $APP_NAME --open-url http://localhost:8080 ``` +> **macOS note:** On macOS with the Application Firewall enabled, a dialog will appear asking +> *"Do you want the application 'cf-cli-java-plugin' to accept incoming network connections?"* +> Click **Allow** — the plugin binds a temporary local server on `127.0.0.1` to serve the file +> to the browser. The server shuts down automatically after the browser fetches the file once. + Getting a thread dump: ```sh diff --git a/docs/superpowers/specs/2026-09-17-open-heap-dump-design.md b/docs/superpowers/specs/2026-09-17-open-heap-dump-design.md index 40eb0ed..4105485 100644 --- a/docs/superpowers/specs/2026-09-17-open-heap-dump-design.md +++ b/docs/superpowers/specs/2026-09-17-open-heap-dump-design.md @@ -81,6 +81,18 @@ Would open: https://parttimenerd.github.io/hprof-analyzer/?file=http://localhost | `--open` + `--no-download` | Error at parse time | | `--open` on non-heap-dump command | Error at parse time | +## Platform notes + +**macOS Application Firewall:** When the macOS firewall is enabled (System Settings → Network → Firewall), the OS +shows a dialog on first use: *"Do you want the application 'cf-cli-java-plugin' to accept incoming network +connections?"* The user must click **Allow**. The plugin binds only to `127.0.0.1` (loopback), so allowing this +does not expose any port to the network. The server exits after the browser fetches the file once. + +**Browser mixed-content:** Chrome and Firefox allow HTTPS pages to fetch from `http://localhost` without any +security warning — `localhost` is treated as a secure context by the browser specifications ( +[W3C Secure Contexts](https://www.w3.org/TR/secure-contexts/#is-origin-trustworthy)). No browser permission +prompt is shown for the fetch itself. + ## Dependencies No new external dependencies. Uses `net/http`, `os/exec`, `runtime` from stdlib. From 0791be02e97248c479b842affbf262e135006942 Mon Sep 17 00:00:00 2001 From: Johannes Bechberger Date: Thu, 17 Sep 2026 16:01:18 +0200 Subject: [PATCH 17/39] docs: add .tool.yaml with --open, --redact, --compress how-to entries --- .tool.yaml | 225 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 225 insertions(+) create mode 100644 .tool.yaml diff --git a/.tool.yaml b/.tool.yaml new file mode 100644 index 0000000..1b4084c --- /dev/null +++ b/.tool.yaml @@ -0,0 +1,225 @@ +tag: ready +github_url: https://github.com/SAP/cf-cli-java-plugin +tagline: Cloud Foundry CLI plugin to troubleshoot Java apps running on CF without SSH. Trigger heap + dumps, thread dumps, and async-profiler or JFR recordings from the cf command line, with results + streamed back to your machine. Also embeds jstall for full JVM inspection via `cf java jstall`. +tagline_short: Trigger heap dumps, thread dumps, and profiles from the CF CLI — no SSH needed. +when_to_use: +- You run Java apps on Cloud Foundry and need heap dumps, thread dumps, or CPU profiles +- You want jstall-style JVM inspection without SSH access to the container +- You need to record JVM diagnostic data (status zip) for offline analysis +when_not_to_use: +- You are not using Cloud Foundry (use jstall directly for non-CF JVMs) +- You need real-time continuous profiling rather than one-shot diagnostics +related: +- label: jstall + url: https://github.com/parttimenerd/jstall +links: +- label: CF Plugin Registry ↗ + url: https://plugins.cloudfoundry.org/ +install: +- label: Latest (manual) + lang: bash + code: | + # Pick the binary for your platform: + cf install-plugin https://github.com/SAP/cf-cli-java-plugin/releases/latest/download/cf-cli-java-plugin-macos-arm64 + # linux-amd64 / linux-arm64 / windows-amd64 / windows-arm64 also available +- label: CF Community + lang: bash + code: | + cf install-plugin -r CF-Community "java" + # Note: community repo lags behind GitHub releases +usage: +- label: Quick start + lang: bash + code: | + cf java heap-dump my-app + cf java thread-dump my-app + cf java jstall my-app +how_to: +- title: My CF app is not responding — find what it is stuck on + body: | + Run `jstall` first — it detects deadlocks, identifies BLOCKED threads, and shows what + each thread is waiting for: + ```bash + cf java jstall $APP_NAME + ``` + If there is a deadlock, it will be listed at the top with the cycle of threads and monitors. + For a focused deadlock check only: + ```bash + cf java jstall $APP_NAME --args 'deadlock all' + ``` + If no deadlock, look for threads in `BLOCKED` state and the monitor they are waiting on. + The thread that *holds* that monitor is the bottleneck. For a plain thread dump: + ```bash + cf java thread-dump $APP_NAME + ``` + +- title: My CF app is using too much CPU + body: | + `most-work` takes repeated thread dumps and ranks threads by on-CPU frequency — + no async-profiler needed: + ```bash + cf java jstall $APP_NAME --args 'most-work --dumps 5 all' + ``` + For a proper CPU flame graph (slower, but much more detail): + ```bash + cf java jstall $APP_NAME --args 'flame all' + # Downloads an HTML flamegraph to your current directory + ``` + Or use the two-step async-profiler approach if you want to capture during a specific window: + ```bash + cf java asprof-start-cpu $APP_NAME + # reproduce the slow operation or wait 30–60 s + cf java asprof-stop $APP_NAME + # Downloads $APP_NAME-asprof-.jfr — open in JDK Mission Control + ``` + +- title: My CF app crashed with OutOfMemoryError — take a heap dump + body: | + Take a heap dump from the running (or restarted) instance and download it: + ```bash + cf java heap-dump $APP_NAME + ``` + Analyse with hprof-analyzer for Leak Suspects and Top Consumers: + ```bash + cf java heap-dump $APP_NAME --open + # Opens hprof-analyzer in the browser with the dump pre-loaded + ``` + Or download and open manually: + ```bash + hprof-analyzer $APP_NAME-heapdump-*.hprof report.html + # Open report.html → "Leak Suspects" and "Top Consumers" tabs + ``` + **Note:** requires jmap, which is not bundled by default in the CF Java Buildpack. + Add a full JDK via `JBP_CONFIG_OPEN_JDK_JRE: '[jre: {version: 21.+}, jdk: {include: true}]'` + to your app's environment if you see a "jmap not found" error. + +- title: Take a heap dump from a running CF app + body: | + ```bash + cf java heap-dump $APP_NAME + ``` + Downloads `$APP_NAME-heapdump-.hprof` to your current directory. + Open it in VisualVM, Eclipse MAT, IntelliJ's heap analyzer, or directly in the browser: + ```bash + cf java heap-dump $APP_NAME --open + # Spins up a local server and opens hprof-analyzer in the browser automatically. + # On macOS with the Application Firewall enabled, click Allow when prompted. + # In Firefox, allow the page to access local services when prompted. + ``` + To remove sensitive data (passwords, tokens) before saving: + ```bash + cf java heap-dump $APP_NAME --redact # lean: zeros primitive arrays + cf java heap-dump $APP_NAME --redact-complete # complete: zeros all primitive values + ``` + To reduce file size (requires JDK 17+ on the container): + ```bash + cf java heap-dump $APP_NAME --compress # saves as .hprof.gz + ``` + **Note:** requires jmap, which is not bundled by default in the CF Java Buildpack. + Add a full JDK via `JBP_CONFIG_OPEN_JDK_JRE: '[jre: {version: 21.+}, jdk: {include: true}]'` + to your app's environment if you see a "jmap not found" error. + +- title: Get a thread dump and spot deadlocks + body: | + ```bash + cf java thread-dump $APP_NAME + ``` + Prints the full thread dump to stdout. To save it: + ```bash + cf java thread-dump $APP_NAME > thread-dump.txt + ``` + For a richer analysis including deadlock detection, hot threads, and lock graphs, use jstall: + ```bash + cf java jstall $APP_NAME --args 'deadlock all' + ``` + +- title: Profile CPU usage with async-profiler + body: | + ```bash + cf java asprof-start-cpu $APP_NAME + # reproduce the slow operation or wait 30–60 s + cf java asprof-stop $APP_NAME + # Downloads $APP_NAME-asprof-.jfr + ``` + Open the `.jfr` file in JDK Mission Control or IntelliJ to view the flame graph. + For a one-shot flame graph without manual start/stop, use jstall: + ```bash + cf java jstall $APP_NAME --args 'flame all' + ``` + +- title: Record a full diagnostic snapshot for offline analysis + body: | + ```bash + # Record everything (thread dump, heap histogram, jcmd output) into a zip: + cf java record-status $APP_NAME + + # Include JFR recording and flame graph (slower, larger): + cf java record-status $APP_NAME --full + + # Replay the zip locally with jstall: + jstall -f $APP_NAME-status.zip status all + jstall -f $APP_NAME-status.zip threads all + ``` + Useful for sharing diagnostics with teammates or filing bug reports without + giving them CF access. + +- title: Inspect a Cloud Foundry app with jstall + body: | + The plugin embeds jstall directly — no separate installation needed: + ```bash + # Full status report (deadlock detection, hot threads, etc.): + cf java jstall $APP_NAME + + # Run a specific jstall subcommand: + cf java jstall $APP_NAME --args 'most-work --dumps 3 all' + cf java jstall $APP_NAME --args 'flame all' + ``` + To use a newer jstall version than the one bundled in the plugin, + use jstall's own `--cf` option instead: + ```bash + jstall --cf $APP_NAME status all + ``` + +- title: Redact sensitive data from a heap dump + body: | + The plugin can zero out sensitive values (passwords, tokens, personal data) before + saving the dump locally, using the bundled hprof-redact tool: + ```bash + # Lean redaction — zeros primitive arrays (byte[], char[], etc.): + cf java heap-dump $APP_NAME --redact + + # Complete redaction — zeros all primitive arrays and individual primitive fields: + cf java heap-dump $APP_NAME --redact-complete + + # Redact and compress: + cf java heap-dump $APP_NAME --redact --compress + + # Redact and open in browser: + cf java heap-dump $APP_NAME --redact --open + ``` + The unredacted dump is never written to disk — redaction happens in-memory during download. + +- title: Open a heap dump in hprof-analyzer + body: | + After downloading, the plugin can spin up a temporary local server and open + [hprof-analyzer](https://parttimenerd.github.io/hprof-analyzer) in the browser automatically: + ```bash + cf java heap-dump $APP_NAME --open + ``` + The server binds to `127.0.0.1` (loopback only), serves the file exactly once under a + random token URL, then shuts down automatically after the browser fetches it. + + **macOS:** if the Application Firewall is enabled, click **Allow** when asked whether + `cf-cli-java-plugin` may accept incoming network connections. + + **Firefox:** click **Allow** when the browser asks for permission to access local services. + + To use a locally running hprof-analyzer instance instead of the hosted one: + ```bash + cf java heap-dump $APP_NAME --open-url http://localhost:8080 + ``` + +note: Requires cf ssh to be enabled on the app (`cf enable-ssh my-app`, then restart). + The heap-dump command additionally needs jmap — see the How To entry above if it is missing. From 812fe9b99af4e81a755d44037956fa4954e2807f Mon Sep 17 00:00:00 2001 From: Johannes Bechberger Date: Thu, 17 Sep 2026 16:22:09 +0200 Subject: [PATCH 18/39] chore: exclude docs/superpowers planning artifacts from repo --- .gitignore | 5 +- .../plans/2026-09-17-open-heap-dump.md | 590 ------------------ .../specs/2026-09-17-open-heap-dump-design.md | 98 --- 3 files changed, 4 insertions(+), 689 deletions(-) delete mode 100644 docs/superpowers/plans/2026-09-17-open-heap-dump.md delete mode 100644 docs/superpowers/specs/2026-09-17-open-heap-dump-design.md diff --git a/.gitignore b/.gitignore index 83d268c..2a18fd0 100644 --- a/.gitignore +++ b/.gitignore @@ -53,4 +53,7 @@ test/snapshots/ dist # go -pkg \ No newline at end of file +pkg + +# Internal planning docs (superpowers skill artifacts) +docs/superpowers/ \ No newline at end of file diff --git a/docs/superpowers/plans/2026-09-17-open-heap-dump.md b/docs/superpowers/plans/2026-09-17-open-heap-dump.md deleted file mode 100644 index 6facd7a..0000000 --- a/docs/superpowers/plans/2026-09-17-open-heap-dump.md +++ /dev/null @@ -1,590 +0,0 @@ -# `--open` Flag for heap-dump Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Add `--open` and `--open-url` flags to `heap-dump` that spin up a single-serve local HTTP server and open the hprof-analyzer web app with `?file=http://localhost:PORT/dump.hprof` after the final local file is saved. - -**Architecture:** A new `open.go` file provides two functions — `serveFileOnce` (binds `:0`, serves the file once, returns port + done channel) and `openBrowser` (cross-platform browser launch). The main plugin wiring in `cf_cli_java_plugin.go` adds two flags, two `Options` fields, parse-time validation, and a call to the open logic after `localFileFullPath` is finalized. - -**Tech Stack:** Go stdlib (`net/http`, `os/exec`, `runtime`, `path/filepath`). No new dependencies. - ---- - -## File Map - -| File | Action | Responsibility | -|------|--------|----------------| -| `open.go` | **Create** | `serveFileOnce` + `openBrowser` | -| `cf_cli_java_plugin.go` | **Modify** | Flag constants, `Options` fields, `flagDefinitions` entries, `parseOptions` wiring + validation, post-download `--open` call | - ---- - -### Task 1: `open.go` — `serveFileOnce` and `openBrowser` - -**Files:** -- Create: `open.go` - -- [ ] **Step 1: Write the failing test** - -Create `open_test.go`: - -```go -package main - -import ( - "fmt" - "io" - "net/http" - "os" - "path/filepath" - "strings" - "testing" -) - -func TestServeFileOnce(t *testing.T) { - // Create a temp file with known content - tmp := t.TempDir() - p := filepath.Join(tmp, "test.hprof") - if err := os.WriteFile(p, []byte("HEAP_CONTENT"), 0o600); err != nil { - t.Fatal(err) - } - - port, done, err := serveFileOnce(p) - if err != nil { - t.Fatalf("serveFileOnce: %v", err) - } - if port <= 0 { - t.Fatalf("expected positive port, got %d", port) - } - - url := fmt.Sprintf("http://localhost:%d/test.hprof", port) - resp, err := http.Get(url) //nolint:noctx - if err != nil { - t.Fatalf("GET %s: %v", url, err) - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - t.Fatalf("expected 200, got %d", resp.StatusCode) - } - if ct := resp.Header.Get("Content-Type"); ct != "application/octet-stream" { - t.Errorf("Content-Type: want application/octet-stream, got %s", ct) - } - if acao := resp.Header.Get("Access-Control-Allow-Origin"); acao != "*" { - t.Errorf("Access-Control-Allow-Origin: want *, got %s", acao) - } - body, _ := io.ReadAll(resp.Body) - if string(body) != "HEAP_CONTENT" { - t.Errorf("body: want HEAP_CONTENT, got %s", body) - } - - // done channel must close after the request completes - select { - case <-done: - default: - t.Error("done channel not closed after successful GET") - } -} - -func TestServeFileOnce_MissingFile(t *testing.T) { - _, _, err := serveFileOnce("/nonexistent/path/dump.hprof") - if err == nil { - t.Fatal("expected error for missing file, got nil") - } -} - -func TestBuildOpenURL(t *testing.T) { - cases := []struct { - base string - port int - filename string - want string - }{ - { - "https://parttimenerd.github.io/hprof-analyzer", - 54321, - "myapp-heapdump-abc.hprof", - "https://parttimenerd.github.io/hprof-analyzer/?file=http://localhost:54321/myapp-heapdump-abc.hprof", - }, - { - "https://parttimenerd.github.io/hprof-analyzer/", - 9000, - "dump.hprof.gz", - "https://parttimenerd.github.io/hprof-analyzer/?file=http://localhost:9000/dump.hprof.gz", - }, - } - for _, tc := range cases { - got := buildOpenURL(tc.base, tc.port, tc.filename) - if got != tc.want { - t.Errorf("buildOpenURL(%q, %d, %q)\n want %q\n got %q", tc.base, tc.port, tc.filename, tc.want, got) - } - } -} - -func TestBuildOpenURL_TrailingSlash(t *testing.T) { - // base with trailing slash must not produce double slash before ? - url := buildOpenURL("https://example.com/analyzer/", 1234, "dump.hprof") - if strings.Contains(url, "//?" ) { - t.Errorf("double slash before ?: %s", url) - } -} -``` - -- [ ] **Step 2: Run tests to verify they fail** - -```bash -go test -run 'TestServeFileOnce|TestBuildOpenURL' ./... 2>&1 -``` - -Expected: `undefined: serveFileOnce`, `undefined: buildOpenURL` - -- [ ] **Step 3: Implement `open.go`** - -```go -/* - * Copyright (c) 2024 SAP SE or an SAP affiliate company. All rights reserved. - * This file is licensed under the Apache Software License, v. 2 except as noted - * otherwise in the LICENSE file at the root of the repository. - */ - -package main - -import ( - "context" - "fmt" - "net" - "net/http" - "os" - "os/exec" - "path/filepath" - "runtime" - "strings" -) - -// serveFileOnce starts a local HTTP server on a random port that serves path -// exactly once. It returns the bound port and a channel that is closed when -// the first GET request completes. The server shuts itself down after serving. -// Returns an error if the file does not exist or the port cannot be bound. -func serveFileOnce(path string) (int, <-chan struct{}, error) { - if _, err := os.Stat(path); err != nil { - return 0, nil, fmt.Errorf("file not found: %w", err) - } - - ln, err := net.Listen("tcp", "127.0.0.1:0") - if err != nil { - return 0, nil, fmt.Errorf("could not bind local port: %w", err) - } - port := ln.Addr().(*net.TCPAddr).Port - - done := make(chan struct{}) - mux := http.NewServeMux() - srv := &http.Server{Handler: mux} - - mux.HandleFunc("/"+filepath.Base(path), func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Access-Control-Allow-Origin", "*") - w.Header().Set("Content-Type", "application/octet-stream") - http.ServeFile(w, r, path) - go func() { - close(done) - _ = srv.Shutdown(context.Background()) - }() - }) - - go func() { _ = srv.Serve(ln) }() - - return port, done, nil -} - -// buildOpenURL constructs the hprof-analyzer URL with the ?file= parameter. -// base is the analyzer base URL (trailing slash optional). -func buildOpenURL(base string, port int, filename string) string { - base = strings.TrimRight(base, "/") - return fmt.Sprintf("%s/?file=http://localhost:%d/%s", base, port, filename) -} - -// openBrowser opens url in the system default browser. -// If launch fails, it prints the URL to stdout so the user can open it manually. -func openBrowser(url string) error { - var cmd *exec.Cmd - switch runtime.GOOS { - case "darwin": - cmd = exec.Command("open", url) - case osWindows: - cmd = exec.Command("cmd", "/c", "start", url) - default: - cmd = exec.Command("xdg-open", url) - } - if err := cmd.Start(); err != nil { - fmt.Printf("Opening: %s\n", url) - return nil - } - return nil -} -``` - -- [ ] **Step 4: Run tests to verify they pass** - -```bash -go test -run 'TestServeFileOnce|TestBuildOpenURL' -v ./... 2>&1 -``` - -Expected: all PASS - -- [ ] **Step 5: Commit** - -```bash -git add open.go open_test.go -git commit -m "feat: add serveFileOnce and openBrowser helpers for --open" -``` - ---- - -### Task 2: Flag constants, Options fields, flagDefinitions - -**Files:** -- Modify: `cf_cli_java_plugin.go` - -- [ ] **Step 1: Write the failing test** - -Add to `cf_cli_java_plugin_test.go` (or create it if it doesn't exist — check with `ls *_test.go`): - -```go -func TestParseOptions_Open(t *testing.T) { - p := &JavaPlugin{} - - // --open sets Open=true and OpenURL to default - opts, _, err := p.parseOptions([]string{"heap-dump", "myapp", "--open"}) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if !opts.Open { - t.Error("expected Open=true") - } - if opts.OpenURL != defaultOpenURL { - t.Errorf("expected OpenURL=%q, got %q", defaultOpenURL, opts.OpenURL) - } -} - -func TestParseOptions_OpenURL_ImpliesOpen(t *testing.T) { - p := &JavaPlugin{} - opts, _, err := p.parseOptions([]string{"heap-dump", "myapp", "--open-url", "http://localhost:8080"}) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if !opts.Open { - t.Error("--open-url should imply Open=true") - } - if opts.OpenURL != "http://localhost:8080" { - t.Errorf("unexpected OpenURL: %q", opts.OpenURL) - } -} - -func TestParseOptions_Open_NoDownload_Error(t *testing.T) { - p := &JavaPlugin{} - _, _, err := p.parseOptions([]string{"heap-dump", "myapp", "--open", "--no-download"}) - if err == nil { - t.Fatal("expected error for --open + --no-download, got nil") - } -} -``` - -- [ ] **Step 2: Run tests to verify they fail** - -```bash -go test -run 'TestParseOptions_Open' -v ./... 2>&1 -``` - -Expected: `undefined: defaultOpenURL` or field `Open` not found in `Options` - -- [ ] **Step 3: Add constants, Options fields, flagDefinitions entries** - -In `cf_cli_java_plugin.go`: - -**Add constants** (after `flagCompress = "compress"` on line ~42): - -```go -flagOpen = "open" -flagOpenURL = "open-url" -defaultOpenURL = "https://parttimenerd.github.io/hprof-analyzer" -``` - -**Add fields to `Options` struct** (after `Compress bool` on line ~218): - -```go -Open bool -OpenURL string -``` - -**Add flag definitions** (append after the `flagCompress` entry in `flagDefinitions`, before the closing `}`): - -```go -{ - Name: flagOpen, - Usage: "open the heap dump in the hprof-analyzer web app after downloading", - Type: typeBool, -}, -{ - Name: flagOpenURL, - Usage: "base URL of the hprof-analyzer instance to open (implies --open)", - Type: typeString, -}, -``` - -- [ ] **Step 4: Wire up in `parseOptions`** - -In the `options := &Options{...}` block (after `Compress:` line ~382), add: - -```go -Open: commandFlags.IsSet(flagOpen) || commandFlags.IsSet(flagOpenURL), -OpenURL: func() string { - if u := commandFlags.String(flagOpenURL); u != "" { - return u - } - return defaultOpenURL -}(), -``` - -Add validation after the existing `Redact && RedactComplete` check (after line ~389): - -```go -if options.Open && options.NoDownload { - return nil, nil, &InvalidUsageError{ - message: "Error: flag '--open' requires a local file and cannot be used with '--no-download'", - } -} -``` - -- [ ] **Step 5: Run tests to verify they pass** - -```bash -go test -run 'TestParseOptions_Open' -v ./... 2>&1 -``` - -Expected: all PASS - -- [ ] **Step 6: Verify build is clean** - -```bash -go build ./... 2>&1 -``` - -Expected: no output (success) - -- [ ] **Step 7: Commit** - -```bash -git add cf_cli_java_plugin.go cf_cli_java_plugin_test.go -git commit -m "feat: add --open and --open-url flags to Options and flagDefinitions" -``` - ---- - -### Task 3: Call open logic after file is finalized - -**Files:** -- Modify: `cf_cli_java_plugin.go` (the post-download section, ~lines 1278-1297) - -- [ ] **Step 1: Write the failing test** - -Add to `cf_cli_java_plugin_test.go`: - -```go -func TestParseOptions_Open_DryRun_NoError(t *testing.T) { - // --open + --dry-run is valid (no server started, just prints URL) - p := &JavaPlugin{} - opts, _, err := p.parseOptions([]string{"heap-dump", "myapp", "--open", "--dry-run"}) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if !opts.Open { - t.Error("expected Open=true") - } - if !opts.DryRun { - t.Error("expected DryRun=true") - } -} -``` - -- [ ] **Step 2: Run to verify it passes immediately** (it's a parse-only test, should pass after Task 2) - -```bash -go test -run 'TestParseOptions_Open_DryRun_NoError' -v ./... 2>&1 -``` - -Expected: PASS - -- [ ] **Step 3: Add the open call after file finalization** - -Locate the block in `cf_cli_java_plugin.go` that ends with redaction (around line 1296). The current structure is: - -```go -fmt.Println(utils.ToSentenceCase(command.FileLabel) + " file saved to: " + localFileFullPath) - -if command.Name == cmdHeapDump && (options.Redact || options.RedactComplete) { - // ... redaction sets finalPath ... - fmt.Println("Redacted heap dump saved to: " + finalPath) -} -``` - -Replace that block with: - -```go -fmt.Println(utils.ToSentenceCase(command.FileLabel) + " file saved to: " + localFileFullPath) - -finalLocalPath := localFileFullPath -if command.Name == cmdHeapDump && (options.Redact || options.RedactComplete) { - mode := "lean" - if options.RedactComplete { - mode = "complete" - } - redactBin, rerr := ensureHprofRedact() - if rerr != nil { - return "", fmt.Errorf("hprof-redact unavailable: %w", rerr) - } - localIsGz := strings.HasSuffix(localFileFullPath, ".hprof.gz") - finalPath, rerr := pipeHeapDumpThroughRedact(redactBin, localFileFullPath, mode, options.Compress || localIsGz) - if rerr != nil { - return "", fmt.Errorf("redaction failed (unredacted file at %s): %w", localFileFullPath, rerr) - } - fmt.Println("Redacted heap dump saved to: " + finalPath) - finalLocalPath = finalPath -} - -if command.Name == cmdHeapDump && options.Open { - if options.DryRun { - fmt.Printf("Would open: %s\n", buildOpenURL(options.OpenURL, 0, filepath.Base(finalLocalPath))) - } else { - port, done, serveErr := serveFileOnce(finalLocalPath) - if serveErr != nil { - return "", fmt.Errorf("could not start local file server: %w", serveErr) - } - openURL := buildOpenURL(options.OpenURL, port, filepath.Base(finalLocalPath)) - fmt.Printf("Opening heap dump in browser: %s\n", openURL) - _ = openBrowser(openURL) - <-done - } -} -``` - -Note: the dry-run `PORT` placeholder uses `0` — `buildOpenURL` with port `0` produces `localhost:0` which clearly signals a placeholder. Update `buildOpenURL` in `open.go` to produce `PORT` when port is 0: - -```go -func buildOpenURL(base string, port int, filename string) string { - base = strings.TrimRight(base, "/") - portStr := fmt.Sprintf("%d", port) - if port == 0 { - portStr = "PORT" - } - return fmt.Sprintf("%s/?file=http://localhost:%s/%s", base, portStr, filename) -} -``` - -Also update the test in `open_test.go` for `buildOpenURL` — add a dry-run case: - -```go -{ - "https://parttimenerd.github.io/hprof-analyzer", - 0, - "dump.hprof", - "https://parttimenerd.github.io/hprof-analyzer/?file=http://localhost:PORT/dump.hprof", -}, -``` - -Also add `"path/filepath"` to the import in `cf_cli_java_plugin.go` if not already present. - -- [ ] **Step 4: Verify build is clean** - -```bash -go build ./... 2>&1 -``` - -Expected: no output - -- [ ] **Step 5: Run all tests** - -```bash -go test -v ./... 2>&1 -``` - -Expected: all PASS - -- [ ] **Step 6: Commit** - -```bash -git add cf_cli_java_plugin.go open.go open_test.go -git commit -m "feat: invoke serveFileOnce+openBrowser after heap-dump download for --open" -``` - ---- - -### Task 4: Build, install, and smoke-test against a live CF app - -**Files:** none (verification only) - -- [ ] **Step 1: Build and reinstall the plugin** - -```bash -go build -o build/cf-cli-java-plugin . && cf install-plugin -f build/cf-cli-java-plugin -``` - -Expected: `Plugin java X.X.X successfully installed.` - -- [ ] **Step 2: Dry-run smoke test** - -```bash -cf java heap-dump sapmachine21 --open --dry-run -``` - -Expected output contains: -``` -Would open: https://parttimenerd.github.io/hprof-analyzer/?file=http://localhost:PORT/sapmachine21-heapdump-.hprof -``` - -- [ ] **Step 3: Dry-run with custom URL** - -```bash -cf java heap-dump sapmachine21 --open-url http://localhost:8080 --dry-run -``` - -Expected: URL starts with `http://localhost:8080/?file=http://localhost:PORT/` - -- [ ] **Step 4: Dry-run `--open` + `--no-download` errors** - -```bash -cf java heap-dump sapmachine21 --open --no-download --dry-run 2>&1; echo "EXIT: $?" -``` - -Expected: error message `'--open' requires a local file and cannot be used with '--no-download'`, exit 1 - -- [ ] **Step 5: Dry-run `--open` + `--compress`** - -```bash -cf java heap-dump sapmachine21 --open --compress --dry-run -``` - -Expected: URL path ends with `.hprof.gz` - -- [ ] **Step 6: Dry-run `--open` + `--redact`** - -```bash -cf java heap-dump sapmachine21 --open --redact --dry-run -``` - -Expected: URL path ends with `-redacted.hprof` - -- [ ] **Step 7: Commit smoke-test confirmation (no code change needed)** - -If all above pass with no code changes required: -```bash -git commit --allow-empty -m "test: smoke-tested --open flag against live CF app" -``` - ---- - -## Self-Review Notes - -- **Spec coverage:** All requirements covered — `--open`, `--open-url`, `--open-url` implies `--open`, `--open` + `--no-download` error, compatible with `--redact`/`--compress`, dry-run prints placeholder URL, serve-once lifecycle, cross-platform `openBrowser`. -- **Placeholder scan:** All code blocks are complete and runnable. -- **Type consistency:** `serveFileOnce` returns `(int, <-chan struct{}, error)` in Task 1 and is called identically in Task 3. `buildOpenURL(base string, port int, filename string) string` is defined in Task 1 and used the same way in Task 3. -- **`filepath` import:** Task 3 notes to add `"path/filepath"` — check if already imported before adding. -- **`finalLocalPath` variable:** Introduced in Task 3 to track the post-redact path; the existing `localFileFullPath` variable is preserved unchanged for the error-path retry message at line ~1303. diff --git a/docs/superpowers/specs/2026-09-17-open-heap-dump-design.md b/docs/superpowers/specs/2026-09-17-open-heap-dump-design.md deleted file mode 100644 index 4105485..0000000 --- a/docs/superpowers/specs/2026-09-17-open-heap-dump-design.md +++ /dev/null @@ -1,98 +0,0 @@ -# Design: `--open` flag for heap-dump - -**Date:** 2026-09-17 -**Branch:** heap-dump-compress-redact -**Status:** Approved - -## Summary - -Add `--open` to the `heap-dump` command. After the final local file is saved (including any redaction or compression), the plugin spins up a temporary single-serve HTTP server, then opens the browser to the hprof-analyzer web app with a `?file=` URL pointing at that server. The server shuts down after the browser fetches the file once. - -The hprof-analyzer web app (`parttimenerd/hprof-analyzer`) is being updated in parallel to support `?file=URL` loading. - -## Flags - -| Flag | Type | Default | Description | -|------|------|---------|-------------| -| `--open` | bool | false | After download, open the heap dump in the hprof-analyzer web app | -| `--open-url` | string | `https://parttimenerd.github.io/hprof-analyzer` | Base URL of the hprof-analyzer web instance to open | - -**Validation rules:** -- `--open` is only valid with `heap-dump`; all other commands error at parse time -- `--open-url` implies `--open` -- `--open` + `--no-download` errors at parse time (no local file to serve) -- `--open` is compatible with `--redact`, `--redact-complete`, `--compress` — always opens the final output file - -## Options struct changes - -```go -// in Options struct -Open bool -OpenURL string -``` - -`OpenURL` defaults to `"https://parttimenerd.github.io/hprof-analyzer"` always (set at options-parse time, not conditionally on `--open`), so `--open-url` alone works without requiring `--open` to be explicitly set. - -## Execution flow - -After `localFileFullPath` is finalized (post-download, post-redact/compress): - -1. Bind `net/http` listener on `:0` (OS assigns free port) -2. Register a single handler that: - - Sets `Access-Control-Allow-Origin: *` (cross-origin fetch from the web app) - - Sets `Content-Type: application/octet-stream` - - Streams the file - - Signals a `done` channel after the response is written -3. Open browser to `{OpenURL}/?file=http://localhost:{PORT}/{basename}` - - macOS: `open ` - - Linux: `xdg-open ` - - Windows: `start ` -4. Block until `done` fires, then shut down the server and return - -## New file: `open.go` - -Two functions: - -```go -// serveFileOnce binds a random port, serves path exactly once, returns the -// port and a channel that closes when the request completes. -func serveFileOnce(path string) (port int, done <-chan struct{}, err error) - -// openBrowser opens url in the default system browser. -// If the browser cannot be launched, it prints the URL to stdout instead. -func openBrowser(url string) error -``` - -## Dry-run behaviour - -Print the URL that would be opened with a `PORT` placeholder instead of a real port: - -``` -Would open: https://parttimenerd.github.io/hprof-analyzer/?file=http://localhost:PORT/sapmachine21-heapdump-abc123.hprof -``` - -## Error handling - -| Scenario | Behaviour | -|----------|-----------| -| Port binding fails | Return error, abort command | -| Browser launch fails | Print URL to stdout, continue blocking for fetch | -| User Ctrl+Cs before fetch | Normal signal handling; plugin exits, server stops | -| `--open` + `--no-download` | Error at parse time | -| `--open` on non-heap-dump command | Error at parse time | - -## Platform notes - -**macOS Application Firewall:** When the macOS firewall is enabled (System Settings → Network → Firewall), the OS -shows a dialog on first use: *"Do you want the application 'cf-cli-java-plugin' to accept incoming network -connections?"* The user must click **Allow**. The plugin binds only to `127.0.0.1` (loopback), so allowing this -does not expose any port to the network. The server exits after the browser fetches the file once. - -**Browser mixed-content:** Chrome and Firefox allow HTTPS pages to fetch from `http://localhost` without any -security warning — `localhost` is treated as a secure context by the browser specifications ( -[W3C Secure Contexts](https://www.w3.org/TR/secure-contexts/#is-origin-trustworthy)). No browser permission -prompt is shown for the fetch itself. - -## Dependencies - -No new external dependencies. Uses `net/http`, `os/exec`, `runtime` from stdlib. From 7f9847bbd395c542a4a6519d7720e7fb2fa4cb70 Mon Sep 17 00:00:00 2001 From: Johannes Bechberger Date: Thu, 17 Sep 2026 16:23:33 +0200 Subject: [PATCH 19/39] docs: fix supported platforms list for --redact in CHANGELOG --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d05a6c..b55eb29 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), `cf java jstall APP_NAME`. Requires Java 17+ locally. Supports all jstall subcommands via `--args`. - `heap-dump --redact`: zeros primitive arrays (`byte[]`, `char[]`, etc.) in the downloaded dump before saving (lean redaction mode), using the bundled [hprof-redact](https://github.com/parttimenerd/hprof-analyzer) binary. - Supported on Linux, macOS (Apple Silicon), and Windows. + Supported on Linux (amd64, arm64), macOS (Apple Silicon), and Windows (amd64, arm64). - `heap-dump --redact-complete`: zeros all primitive arrays and individual primitive fields (complete redaction mode, maximum privacy). Mutually exclusive with `--redact`. - `heap-dump --compress`: saves the dump as `.hprof.gz` by transferring it gzip-compressed over SSH (requires JDK 17+ From 160e510265c325f9be3868d25e79251042c7c1dc Mon Sep 17 00:00:00 2001 From: Johannes Bechberger Date: Thu, 17 Sep 2026 16:37:42 +0200 Subject: [PATCH 20/39] docs: clarify that gzip transfer is always used on JDK 17+, --compress only affects local file format --- .tool.yaml | 2 +- CHANGELOG.md | 9 +++++---- README.md | 11 ++++++----- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/.tool.yaml b/.tool.yaml index 1b4084c..d5d4b72 100644 --- a/.tool.yaml +++ b/.tool.yaml @@ -113,7 +113,7 @@ how_to: cf java heap-dump $APP_NAME --redact # lean: zeros primitive arrays cf java heap-dump $APP_NAME --redact-complete # complete: zeros all primitive values ``` - To reduce file size (requires JDK 17+ on the container): + To keep the local file compressed as .hprof.gz (transfer is already gzip-compressed on JDK 17+): ```bash cf java heap-dump $APP_NAME --compress # saves as .hprof.gz ``` diff --git a/CHANGELOG.md b/CHANGELOG.md index b55eb29..fb7b6fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,10 +16,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), Supported on Linux (amd64, arm64), macOS (Apple Silicon), and Windows (amd64, arm64). - `heap-dump --redact-complete`: zeros all primitive arrays and individual primitive fields (complete redaction mode, maximum privacy). Mutually exclusive with `--redact`. -- `heap-dump --compress`: saves the dump as `.hprof.gz` by transferring it gzip-compressed over SSH (requires JDK 17+ - on the container). Prints a warning and falls back to uncompressed on older JDKs. -- Transparent compressed transfer: on JDK 17+ containers, the plugin automatically uses `jmap gz=1` to reduce - transfer size even without `--compress`, decompressing on the fly so the local file is always a plain `.hprof`. +- `heap-dump --compress`: saves the local file as `.hprof.gz` instead of decompressing it after transfer + (requires JDK 17+ on the container). Useful when you want to store or share the compressed dump directly. +- Transparent compressed transfer: on JDK 17+ containers, the plugin always uses `jmap gz=1` to compress + the dump during SSH transfer (faster on slow connections), then decompresses on the fly so the local file + is a plain `.hprof`. Use `--compress` to keep the file compressed locally. - `heap-dump --open`: after downloading (and optionally redacting/compressing) the dump, spins up a temporary local HTTP server and opens the [hprof-analyzer](https://parttimenerd.github.io/hprof-analyzer) web app in the default browser with the dump pre-loaded. The server serves the file exactly once via a random token URL and shuts down diff --git a/README.md b/README.md index 7381ae6..a3953df 100644 --- a/README.md +++ b/README.md @@ -16,7 +16,7 @@ Currently, it allows you to: - Run [jstall](https://github.com/parttimenerd/jstall) for one-shot JVM inspection (deadlock detection, hot threads, dependency graphs, and more): bundled directly in the plugin, requires Java 17+ locally - Redact heap dumps before saving to remove sensitive data (`--redact`, `--redact-complete`) -- Reduce transfer size by compressing heap dumps over SSH (`--compress`) +- Automatically compress heap dump transfers over SSH on JDK 17+ containers; use `--compress` to keep the local file as `.hprof.gz` - Open heap dumps directly in the [hprof-analyzer](https://parttimenerd.github.io/hprof-analyzer) web app after downloading (`--open`) ## Installation @@ -198,17 +198,18 @@ is not in `cf java`, but in whatever makes `cf ssh` fail. Getting a heap dump: ```sh -# Basic — plain .hprof saved locally +# Basic — plain .hprof saved locally. +# On JDK 17+ containers, transfer is always gzip-compressed automatically (faster on slow connections). cf java heap-dump $APP_NAME # Redact sensitive values (passwords, tokens, personal data) before saving cf java heap-dump $APP_NAME --redact # lean: zeros primitive arrays cf java heap-dump $APP_NAME --redact-complete # complete: zeros all primitive values -# Compress the output (JDK 17+ on container required; falls back to uncompressed otherwise) -cf java heap-dump $APP_NAME --compress # saves as .hprof.gz +# Keep the local file compressed as .hprof.gz (transfer is already compressed on JDK 17+) +cf java heap-dump $APP_NAME --compress -# Redact and compress +# Redact and keep compressed cf java heap-dump $APP_NAME --redact --compress # Open in hprof-analyzer web app after downloading (spins up a local server, opens browser) From 189fe2429930433728f6fe9107d3d575ddc97338 Mon Sep 17 00:00:00 2001 From: Johannes Bechberger Date: Thu, 17 Sep 2026 16:42:22 +0200 Subject: [PATCH 21/39] ci: download hprof-redact binaries before golangci-lint in PR validation golangci-lint typecheck fails when the dist/ hprof-redact binaries are missing (go:embed pattern not satisfied). Add a download step mirroring build.py, placed after the existing jstall download and before lint. --- .github/workflows/pr-validation.yml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/.github/workflows/pr-validation.yml b/.github/workflows/pr-validation.yml index e39f333..5a23a71 100644 --- a/.github/workflows/pr-validation.yml +++ b/.github/workflows/pr-validation.yml @@ -38,6 +38,15 @@ jobs: mkdir -p dist curl -fsSL -o dist/jstall-minimal.jar https://github.com/parttimenerd/jstall/releases/latest/download/jstall-minimal.jar + - name: Download hprof-redact binaries for go:embed + run: | + HPROF_BASE="https://github.com/parttimenerd/hprof-analyzer/releases/download/nightly" + curl -fsSL -o dist/hprof-redact-linux-amd64 "$HPROF_BASE/hprof-redact-linux-amd64" + curl -fsSL -o dist/hprof-redact-linux-arm64 "$HPROF_BASE/hprof-redact-linux-arm64" + curl -fsSL -o dist/hprof-redact-darwin-arm64 "$HPROF_BASE/hprof-redact-darwin-arm64" + curl -fsSL -o dist/hprof-redact-windows-amd64.exe "$HPROF_BASE/hprof-redact-windows-amd64.exe" + curl -fsSL -o dist/hprof-redact-windows-arm64.exe "$HPROF_BASE/hprof-redact-windows-arm64.exe" + - name: Install Go dependencies run: go mod tidy -e || true From c6e7ea785ce8d4010e480d07a3f20fb1b9d8f672 Mon Sep 17 00:00:00 2001 From: Johannes Bechberger Date: Thu, 17 Sep 2026 16:47:03 +0200 Subject: [PATCH 22/39] ci: fix hprof-redact download by extracting from archives via build.py The previous attempt tried to curl the binaries directly, but they are packed inside tar.gz/zip archives. Add --deps-only flag to build.py to run only the download+extraction step (no go build), and use it in the PR validation workflow before golangci-lint. --- .github/workflows/build.py | 51 +++++++++++++++++++++++++++-- .github/workflows/pr-validation.yml | 8 +---- 2 files changed, 49 insertions(+), 10 deletions(-) diff --git a/.github/workflows/build.py b/.github/workflows/build.py index 74b8a06..38f4941 100644 --- a/.github/workflows/build.py +++ b/.github/workflows/build.py @@ -1,5 +1,10 @@ import os import platform +import subprocess +import sys +import tarfile +import urllib.request +import zipfile os.makedirs('dist', exist_ok=True) @@ -19,6 +24,46 @@ arch = arch_map[platform.machine().lower()] print(f"Building for {os_name} {arch}") -import sys -rc = os.system(f"go build -o dist/cf-cli-java-plugin-{os_name}-{arch}") -sys.exit(rc >> 8 if os.name != 'nt' else rc) \ No newline at end of file +HPROF_BASE = "https://github.com/parttimenerd/hprof-analyzer/releases/download/nightly" + +# All platform binaries must exist before go build (go:embed requires them all). +hprof_targets = [ + ("hprof-analyzer-x86_64-unknown-linux-musl.tar.gz", "hprof-analyzer-x86_64-unknown-linux-musl/hprof-redact", "dist/hprof-redact-linux-amd64", False), + ("hprof-analyzer-aarch64-unknown-linux-musl.tar.gz", "hprof-analyzer-aarch64-unknown-linux-musl/hprof-redact", "dist/hprof-redact-linux-arm64", False), + ("hprof-analyzer-aarch64-apple-darwin.tar.gz", "hprof-analyzer-aarch64-apple-darwin/hprof-redact", "dist/hprof-redact-darwin-arm64", False), + ("hprof-analyzer-x86_64-pc-windows-msvc.zip", "hprof-analyzer-x86_64-pc-windows-msvc/hprof-redact.exe", "dist/hprof-redact-windows-amd64.exe", True), + ("hprof-analyzer-aarch64-pc-windows-msvc.zip", "hprof-analyzer-aarch64-pc-windows-msvc/hprof-redact.exe", "dist/hprof-redact-windows-arm64.exe", True), +] + +for archive_name, member, dest, is_zip in hprof_targets: + if os.path.exists(dest) and os.path.getsize(dest) > 0: + print(f" {dest} already present, skipping") + continue + url = f"{HPROF_BASE}/{archive_name}" + print(f" Downloading {archive_name} -> {dest}") + tmp = dest + ".tmp" + try: + urllib.request.urlretrieve(url, tmp) + if is_zip: + with zipfile.ZipFile(tmp) as zf: + data = zf.read(member) + else: + with tarfile.open(tmp, "r:gz") as tf: + data = tf.extractfile(member).read() + with open(dest, "wb") as f: + f.write(data) + os.chmod(dest, 0o755) + except Exception as e: + # windows-arm64 may not always have a release; create an empty placeholder + # so go:embed compiles (the plugin will report "not available" at runtime). + print(f" Warning: could not download {archive_name}: {e}; creating empty placeholder") + open(dest, "wb").close() + finally: + if os.path.exists(tmp): + os.remove(tmp) + +if "--deps-only" in sys.argv: + sys.exit(0) + +rc = subprocess.call(f"go build -o dist/cf-cli-java-plugin-{os_name}-{arch}", shell=True) +sys.exit(rc) diff --git a/.github/workflows/pr-validation.yml b/.github/workflows/pr-validation.yml index 5a23a71..7d6ad20 100644 --- a/.github/workflows/pr-validation.yml +++ b/.github/workflows/pr-validation.yml @@ -39,13 +39,7 @@ jobs: curl -fsSL -o dist/jstall-minimal.jar https://github.com/parttimenerd/jstall/releases/latest/download/jstall-minimal.jar - name: Download hprof-redact binaries for go:embed - run: | - HPROF_BASE="https://github.com/parttimenerd/hprof-analyzer/releases/download/nightly" - curl -fsSL -o dist/hprof-redact-linux-amd64 "$HPROF_BASE/hprof-redact-linux-amd64" - curl -fsSL -o dist/hprof-redact-linux-arm64 "$HPROF_BASE/hprof-redact-linux-arm64" - curl -fsSL -o dist/hprof-redact-darwin-arm64 "$HPROF_BASE/hprof-redact-darwin-arm64" - curl -fsSL -o dist/hprof-redact-windows-amd64.exe "$HPROF_BASE/hprof-redact-windows-amd64.exe" - curl -fsSL -o dist/hprof-redact-windows-arm64.exe "$HPROF_BASE/hprof-redact-windows-arm64.exe" + run: python3 .github/workflows/build.py --deps-only - name: Install Go dependencies run: go mod tidy -e || true From fa8c71c1c3bf52b9d95f0c931a7d34dcf1b709ae Mon Sep 17 00:00:00 2001 From: Johannes Bechberger Date: Thu, 17 Sep 2026 16:49:37 +0200 Subject: [PATCH 23/39] style: add trailing newline to .gitignore --- .gitignore | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 2a18fd0..76b4cfa 100644 --- a/.gitignore +++ b/.gitignore @@ -56,4 +56,4 @@ dist pkg # Internal planning docs (superpowers skill artifacts) -docs/superpowers/ \ No newline at end of file +docs/superpowers/ From 332018e651a8ed0d895ccced6b17d131ab3f5bf0 Mon Sep 17 00:00:00 2001 From: Johannes Bechberger Date: Thu, 17 Sep 2026 17:05:21 +0200 Subject: [PATCH 24/39] style: fix markdownlint MD013/MD060 violations in README --- README.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index a3953df..56b1cae 100644 --- a/README.md +++ b/README.md @@ -16,8 +16,10 @@ Currently, it allows you to: - Run [jstall](https://github.com/parttimenerd/jstall) for one-shot JVM inspection (deadlock detection, hot threads, dependency graphs, and more): bundled directly in the plugin, requires Java 17+ locally - Redact heap dumps before saving to remove sensitive data (`--redact`, `--redact-complete`) -- Automatically compress heap dump transfers over SSH on JDK 17+ containers; use `--compress` to keep the local file as `.hprof.gz` -- Open heap dumps directly in the [hprof-analyzer](https://parttimenerd.github.io/hprof-analyzer) web app after downloading (`--open`) +- Automatically compress heap dump transfers over SSH on JDK 17+ containers; + use `--compress` to keep the local file as `.hprof.gz` +- Open heap dumps directly in the [hprof-analyzer](https://parttimenerd.github.io/hprof-analyzer) web app + after downloading (`--open`) ## Installation @@ -364,7 +366,7 @@ hold passwords, tokens, session data, or personal information. Before sharing a use `--redact` or `--redact-complete` to zero out sensitive values. | Flag | What gets zeroed | -|------|-----------------| +|------|------------------| | `--redact` | Primitive arrays (`byte[]`, `char[]`, `int[]`, …) — covers most strings and serialized data | | `--redact-complete` | All primitive arrays **and** individual primitive fields — maximum privacy | From b99e31edf9410f6cb84b262b5038f7d6b0c5c243 Mon Sep 17 00:00:00 2001 From: Johannes Bechberger Date: Thu, 17 Sep 2026 17:09:07 +0200 Subject: [PATCH 25/39] style: fix MD060 aligned table style in README redact section --- README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 56b1cae..688a268 100644 --- a/README.md +++ b/README.md @@ -365,10 +365,10 @@ Heap dumps contain the full in-memory state of a JVM, including strings, byte ar hold passwords, tokens, session data, or personal information. Before sharing a dump outside a trusted environment, use `--redact` or `--redact-complete` to zero out sensitive values. -| Flag | What gets zeroed | -|------|------------------| -| `--redact` | Primitive arrays (`byte[]`, `char[]`, `int[]`, …) — covers most strings and serialized data | -| `--redact-complete` | All primitive arrays **and** individual primitive fields — maximum privacy | +| Flag | What gets zeroed | +| ------------------- | -------------------------------------------------------------------------------------------- | +| `--redact` | Primitive arrays (`byte[]`, `char[]`, `int[]`, …) — covers most strings and serialized data | +| `--redact-complete` | All primitive arrays **and** individual primitive fields — maximum privacy | Both modes preserve the full object graph (class names, references, instance counts), so the dump remains useful for memory analysis. The two flags are mutually exclusive. From 6dcbc211f364509d952e3cd24dc4ab3dcfac2947 Mon Sep 17 00:00:00 2001 From: Johannes Bechberger Date: Thu, 17 Sep 2026 17:13:01 +0200 Subject: [PATCH 26/39] style: fix MD060 table column alignment accounting for multi-byte chars --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 688a268..84ce1f0 100644 --- a/README.md +++ b/README.md @@ -367,7 +367,7 @@ use `--redact` or `--redact-complete` to zero out sensitive values. | Flag | What gets zeroed | | ------------------- | -------------------------------------------------------------------------------------------- | -| `--redact` | Primitive arrays (`byte[]`, `char[]`, `int[]`, …) — covers most strings and serialized data | +| `--redact` | Primitive arrays (`byte[]`, `char[]`, `int[]`, …) — covers most strings and serialized data | | `--redact-complete` | All primitive arrays **and** individual primitive fields — maximum privacy | Both modes preserve the full object graph (class names, references, instance counts), so the dump remains useful for From 06de3c923bc3acff030643327983b4988e3f587c Mon Sep 17 00:00:00 2001 From: Johannes Bechberger Date: Fri, 18 Sep 2026 16:20:35 +0200 Subject: [PATCH 27/39] fix: address PR review comments on heap-dump redact/open features - redact: delete partial output file on failure instead of leaving it behind; add --redact-keep-on-error flag to preserve it when needed - open: use sync.Once to guard close(doneCh)+srv.Shutdown so concurrent GETs cannot panic via double-close - open: print error and manual-open URL when browser launch fails --- cf_cli_java_plugin.go | 112 ++++++++++++++++++++++-------------------- open.go | 11 +++-- redact.go | 7 ++- 3 files changed, 74 insertions(+), 56 deletions(-) diff --git a/cf_cli_java_plugin.go b/cf_cli_java_plugin.go index e41dd86..56458b5 100644 --- a/cf_cli_java_plugin.go +++ b/cf_cli_java_plugin.go @@ -31,29 +31,30 @@ var _ plugin.Plugin = (*JavaPlugin)(nil) // String constants extracted to satisfy goconst linter. const ( - cmdSSH = "ssh" - cmdJava = "java" - flagKeep = "keep" - flagNoDownload = "no-download" - flagContainerDir = "container-dir" - flagLocalDir = "local-dir" - flagRedact = "redact" - flagRedactComplete = "redact-complete" - flagCompress = "compress" - flagOpen = "open" - flagOpenURL = "open-url" - defaultOpenURL = "https://parttimenerd.github.io/hprof-analyzer" - osWindows = "windows" - cmdHeapDump = "heap-dump" - typeBool = "bool" - typeString = "string" - toolJcmd = "jcmd" - toolAsprof = "asprof" - extJFR = ".jfr" - labelJFR = "JFR recording" - partJFR = "jfr" - extHprof = ".hprof" - extHprofGz = ".hprof.gz" + cmdSSH = "ssh" + cmdJava = "java" + flagKeep = "keep" + flagNoDownload = "no-download" + flagContainerDir = "container-dir" + flagLocalDir = "local-dir" + flagRedact = "redact" + flagRedactComplete = "redact-complete" + flagRedactKeepOnError = "redact-keep-on-error" + flagCompress = "compress" + flagOpen = "open" + flagOpenURL = "open-url" + defaultOpenURL = "https://parttimenerd.github.io/hprof-analyzer" + osWindows = "windows" + cmdHeapDump = "heap-dump" + typeBool = "bool" + typeString = "string" + toolJcmd = "jcmd" + toolAsprof = "asprof" + extJFR = ".jfr" + labelJFR = "JFR recording" + partJFR = "jfr" + extHprof = ".hprof" + extHprofGz = ".hprof.gz" ) // JavaPlugin is a CF CLI plugin that supports taking heap and thread dumps on demand @@ -208,20 +209,21 @@ func (c *JavaPlugin) checkSSHConnectivity(appName string, appInstanceIndex int) // Options holds all command-line options for the Java plugin type Options struct { - AppInstanceIndex int - Keep bool - NoDownload bool - DryRun bool - Verbose bool - Full bool - ContainerDir string - LocalDir string - Args string - Redact bool - RedactComplete bool - Compress bool - Open bool - OpenURL string + AppInstanceIndex int + Keep bool + NoDownload bool + DryRun bool + Verbose bool + Full bool + ContainerDir string + LocalDir string + Args string + Redact bool + RedactComplete bool + RedactKeepOnError bool + Compress bool + Open bool + OpenURL string } // FlagDefinition holds metadata for a command-line flag @@ -310,6 +312,11 @@ var flagDefinitions = []FlagDefinition{ Usage: "redact heap dump (complete mode: zero all primitive values) before saving locally", Type: typeBool, }, + { + Name: flagRedactKeepOnError, + Usage: "keep partially-written redacted file if redaction fails (default: delete it)", + Type: typeBool, + }, { Name: flagCompress, Usage: "compress heap dump on container using jmap gz=1 (JDK 17+) to reduce transfer size; output is .hprof.gz", @@ -384,19 +391,20 @@ func (c *JavaPlugin) parseOptions(args []string) (*Options, []string, error) { } options := &Options{ - AppInstanceIndex: appInstanceIndex, - Keep: keep, - NoDownload: noDownload, - DryRun: commandFlags.IsSet("dry-run"), - Verbose: commandFlags.IsSet("verbose"), - Full: commandFlags.IsSet("full"), - ContainerDir: commandFlags.String("container-dir"), - LocalDir: commandFlags.String("local-dir"), - Args: commandFlags.String("args"), - Redact: commandFlags.IsSet(flagRedact), - RedactComplete: commandFlags.IsSet(flagRedactComplete), - Compress: commandFlags.IsSet(flagCompress), - Open: commandFlags.IsSet(flagOpen) || commandFlags.IsSet(flagOpenURL), + AppInstanceIndex: appInstanceIndex, + Keep: keep, + NoDownload: noDownload, + DryRun: commandFlags.IsSet("dry-run"), + Verbose: commandFlags.IsSet("verbose"), + Full: commandFlags.IsSet("full"), + ContainerDir: commandFlags.String("container-dir"), + LocalDir: commandFlags.String("local-dir"), + Args: commandFlags.String("args"), + Redact: commandFlags.IsSet(flagRedact), + RedactComplete: commandFlags.IsSet(flagRedactComplete), + RedactKeepOnError: commandFlags.IsSet(flagRedactKeepOnError), + Compress: commandFlags.IsSet(flagCompress), + Open: commandFlags.IsSet(flagOpen) || commandFlags.IsSet(flagOpenURL), OpenURL: func() string { if u := commandFlags.String(flagOpenURL); u != "" { return u @@ -1326,9 +1334,9 @@ func (c *JavaPlugin) execute(_ plugin.CliConnection, args []string) (string, err return "", fmt.Errorf("hprof-redact unavailable: %w", rerr) } localIsGz := strings.HasSuffix(localFileFullPath, extHprofGz) - finalPath, rerr := pipeHeapDumpThroughRedact(redactBin, localFileFullPath, mode, options.Compress || localIsGz) + finalPath, rerr := pipeHeapDumpThroughRedact(redactBin, localFileFullPath, mode, options.Compress || localIsGz, options.RedactKeepOnError) if rerr != nil { - return "", fmt.Errorf("redaction failed (unredacted file at %s): %w", localFileFullPath, rerr) + return "", fmt.Errorf("redaction failed: %w", rerr) } fmt.Println("Redacted heap dump saved to: " + finalPath) finalLocalPath = finalPath diff --git a/open.go b/open.go index fd7ff40..f1c088a 100644 --- a/open.go +++ b/open.go @@ -18,6 +18,7 @@ import ( "os/exec" "runtime" "strings" + "sync" "time" ) @@ -50,6 +51,7 @@ func serveFileOnce(path string) (port int, urlFile string, done <-chan struct{}, port = ln.Addr().(*net.TCPAddr).Port doneCh := make(chan struct{}) + var shutdownOnce sync.Once mux := http.NewServeMux() srv := &http.Server{ Handler: mux, @@ -80,11 +82,14 @@ func serveFileOnce(path string) (port int, urlFile string, done <-chan struct{}, _, _ = io.Copy(w, f) // Use a fresh context: r.Context() is canceled when the handler returns, // but Shutdown must outlive the request. + // sync.Once ensures concurrent GETs can't double-close doneCh (panic). shutdownCtx, shutdownCancel := context.WithTimeout(context.Background(), 5*time.Second) //nolint:contextcheck go func(ctx context.Context, cancel context.CancelFunc) { //nolint:contextcheck defer cancel() - close(doneCh) - _ = srv.Shutdown(ctx) + shutdownOnce.Do(func() { + close(doneCh) + _ = srv.Shutdown(ctx) + }) }(shutdownCtx, shutdownCancel) }) @@ -118,6 +123,6 @@ func openBrowser(url string) { cmd = exec.Command("xdg-open", url) } if err := cmd.Start(); err != nil { - fmt.Printf("Opening: %s\n", url) + fmt.Printf("Could not open browser (%v). Open manually: %s\n", err, url) } } diff --git a/redact.go b/redact.go index 246a5b7..5d3498a 100644 --- a/redact.go +++ b/redact.go @@ -96,7 +96,7 @@ func ensureHprofRedact() (string, error) { // // mode must be "lean" or "complete". If compress is true the output is written // as .hprof.gz. -func pipeHeapDumpThroughRedact(redactBin, localPath, mode string, compress bool) (string, error) { +func pipeHeapDumpThroughRedact(redactBin, localPath, mode string, compress bool, keepOnError bool) (string, error) { base := strings.TrimSuffix(localPath, ".hprof.gz") base = strings.TrimSuffix(base, ".hprof") var outputPath string @@ -116,6 +116,11 @@ func pipeHeapDumpThroughRedact(redactBin, localPath, mode string, compress bool) cmd.Stderr = os.Stderr if err := cmd.Run(); err != nil { + if !keepOnError { + if rmErr := os.Remove(outputPath); rmErr != nil && !os.IsNotExist(rmErr) { + fmt.Fprintf(os.Stderr, "warning: could not remove partial redacted file %s: %v\n", outputPath, rmErr) + } + } return "", fmt.Errorf("hprof-redact failed: %w", err) } From c95cdd4f0224e6caef7aa0b3848edc885a83174d Mon Sep 17 00:00:00 2001 From: Johannes Bechberger Date: Fri, 18 Sep 2026 16:33:04 +0200 Subject: [PATCH 28/39] feat: make heap-dump open/redact more robust MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit serveFileOnce: - add timeout parameter (10 min in production) — CLI no longer hangs forever if the browser never fetches the dump - add WriteTimeout (30s) to prevent stalled clients holding a goroutine - print stderr message when timeout expires so user knows why CLI exited tests: - TestServeFileOnce_TimeoutClosesServer: done closes, server stops after timeout - TestServeFileOnce_ConcurrentGETsNoPanic: regression for sync.Once fix - TestPipeHeapDumpThroughRedact_ErrorDeletesPartial: partial file removed on failure - TestPipeHeapDumpThroughRedact_KeepOnError: partial file kept with keepOnError=true - TestPipeHeapDumpThroughRedact_HappyPath: success deletes source, returns output path --- cf_cli_java_plugin.go | 3 +- open.go | 24 +++++++++- open_test.go | 77 ++++++++++++++++++++++++++++--- redact_test.go | 102 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 198 insertions(+), 8 deletions(-) create mode 100644 redact_test.go diff --git a/cf_cli_java_plugin.go b/cf_cli_java_plugin.go index 56458b5..9cf7077 100644 --- a/cf_cli_java_plugin.go +++ b/cf_cli_java_plugin.go @@ -16,6 +16,7 @@ import ( "path/filepath" "strconv" "strings" + "time" "code.cloudfoundry.org/cli/cf/terminal" "code.cloudfoundry.org/cli/cf/trace" @@ -1343,7 +1344,7 @@ func (c *JavaPlugin) execute(_ plugin.CliConnection, args []string) (string, err } if command.Name == cmdHeapDump && options.Open { - port, urlFile, done, serveErr := serveFileOnce(finalLocalPath) + port, urlFile, done, serveErr := serveFileOnce(finalLocalPath, 10*time.Minute) if serveErr != nil { return "", fmt.Errorf("could not start local file server: %w", serveErr) } diff --git a/open.go b/open.go index f1c088a..c9cf4de 100644 --- a/open.go +++ b/open.go @@ -26,8 +26,9 @@ import ( // exactly once. The file is exposed under a random token path (e.g. /a3f9c2.hprof) // so the local filename is never leaked and only the holder of the URL can fetch it. // Returns the bound port, the randomised URL path segment, and a channel that -// closes when the first GET request completes. -func serveFileOnce(path string) (port int, urlFile string, done <-chan struct{}, err error) { +// closes when the first GET request completes or timeout elapses. +// timeout 0 means no timeout. +func serveFileOnce(path string, timeout time.Duration) (port int, urlFile string, done <-chan struct{}, err error) { if _, err = os.Stat(path); err != nil { return 0, "", nil, fmt.Errorf("file not found: %w", err) } @@ -56,6 +57,7 @@ func serveFileOnce(path string) (port int, urlFile string, done <-chan struct{}, srv := &http.Server{ Handler: mux, ReadHeaderTimeout: 10 * time.Second, + WriteTimeout: 30 * time.Second, } // Register only the exact random path — any other request gets 404. @@ -95,6 +97,24 @@ func serveFileOnce(path string) (port int, urlFile string, done <-chan struct{}, go func() { _ = srv.Serve(ln) }() + if timeout > 0 { + go func() { + timer := time.NewTimer(timeout) + defer timer.Stop() + select { + case <-doneCh: + case <-timer.C: + shutdownCtx, shutdownCancel := context.WithTimeout(context.Background(), 5*time.Second) //nolint:contextcheck + defer shutdownCancel() + shutdownOnce.Do(func() { + fmt.Fprintf(os.Stderr, "Timed out waiting for browser to fetch heap dump; closing local server.\n") + close(doneCh) + _ = srv.Shutdown(shutdownCtx) + }) + } + }() + } + return port, urlFile, doneCh, nil } diff --git a/open_test.go b/open_test.go index c3322e9..5be145c 100644 --- a/open_test.go +++ b/open_test.go @@ -11,6 +11,34 @@ import ( "time" ) +func TestServeFileOnce_TimeoutClosesServer(t *testing.T) { + tmp := t.TempDir() + p := filepath.Join(tmp, "test.hprof") + if err := os.WriteFile(p, []byte("DATA"), 0o600); err != nil { + t.Fatal(err) + } + + port, urlFile, done, err := serveFileOnce(p, 100*time.Millisecond) + if err != nil { + t.Fatalf("serveFileOnce: %v", err) + } + + // done must close within a reasonable time without any GET + select { + case <-done: + case <-time.After(2 * time.Second): + t.Fatal("done channel not closed after timeout elapsed") + } + + // server must be gone — further requests should fail + url := fmt.Sprintf("http://localhost:%d/%s", port, urlFile) + resp, gerr := http.Get(url) //nolint:noctx,gosec + if gerr == nil { + _ = resp.Body.Close() + t.Error("expected connection refused after server shutdown, but got a response") + } +} + func TestServeFileOnce(t *testing.T) { tmp := t.TempDir() p := filepath.Join(tmp, "test.hprof") @@ -18,7 +46,7 @@ func TestServeFileOnce(t *testing.T) { t.Fatal(err) } - port, urlFile, done, err := serveFileOnce(p) + port, urlFile, done, err := serveFileOnce(p, 30*time.Second) if err != nil { t.Fatalf("serveFileOnce: %v", err) } @@ -73,7 +101,7 @@ func TestServeFileOnce_WrongPath404(t *testing.T) { t.Fatal(err) } - port, _, _, err := serveFileOnce(p) + port, _, _, err := serveFileOnce(p, 30*time.Second) if err != nil { t.Fatalf("serveFileOnce: %v", err) } @@ -99,7 +127,7 @@ func TestServeFileOnce_GzExtension(t *testing.T) { t.Fatal(err) } - _, urlFile, _, err := serveFileOnce(p) + _, urlFile, _, err := serveFileOnce(p, 30*time.Second) if err != nil { t.Fatalf("serveFileOnce: %v", err) } @@ -109,7 +137,7 @@ func TestServeFileOnce_GzExtension(t *testing.T) { } func TestServeFileOnce_MissingFile(t *testing.T) { - _, _, _, err := serveFileOnce("/nonexistent/path/dump.hprof") + _, _, _, err := serveFileOnce("/nonexistent/path/dump.hprof", 30*time.Second) if err == nil { t.Fatal("expected error for missing file, got nil") } @@ -122,7 +150,7 @@ func TestServeFileOnce_OptionsPreflight(t *testing.T) { t.Fatal(err) } - port, urlFile, done, err := serveFileOnce(p) + port, urlFile, done, err := serveFileOnce(p, 30*time.Second) if err != nil { t.Fatalf("serveFileOnce: %v", err) } @@ -201,3 +229,42 @@ func TestBuildOpenURL_TrailingSlash(t *testing.T) { t.Errorf("double slash before ?: %s", url) } } + +func TestServeFileOnce_ConcurrentGETsNoPanic(t *testing.T) { + tmp := t.TempDir() + p := filepath.Join(tmp, "test.hprof") + if err := os.WriteFile(p, []byte("CONCURRENT"), 0o600); err != nil { + t.Fatal(err) + } + + port, urlFile, done, err := serveFileOnce(p, 30*time.Second) + if err != nil { + t.Fatalf("serveFileOnce: %v", err) + } + + url := fmt.Sprintf("http://localhost:%d/%s", port, urlFile) + + // Fire two GETs simultaneously; neither should panic and done must close exactly once. + errs := make(chan error, 2) + for range 2 { + go func() { + resp, gerr := http.Get(url) //nolint:noctx,gosec + if gerr == nil { + _, _ = io.ReadAll(resp.Body) + _ = resp.Body.Close() + } + errs <- gerr + }() + } + + // Collect both results — one may get a connection-refused after server shuts down + for range 2 { + <-errs + } + + select { + case <-done: + case <-time.After(3 * time.Second): + t.Error("done channel not closed after concurrent GETs") + } +} diff --git a/redact_test.go b/redact_test.go new file mode 100644 index 0000000..57d52fe --- /dev/null +++ b/redact_test.go @@ -0,0 +1,102 @@ +package main + +import ( + "fmt" + "os" + "path/filepath" + "testing" +) + +// makeFakeBin writes a shell script that exits with the given code and returns its path. +func makeFakeBin(t *testing.T, exitCode int) string { + t.Helper() + tmp := t.TempDir() + bin := filepath.Join(tmp, "fake-redact") + script := fmt.Sprintf("#!/bin/sh\nexit %d\n", exitCode) + if err := os.WriteFile(bin, []byte(script), 0o755); err != nil { //nolint:gosec // test binary must be executable + t.Fatalf("write fake bin: %v", err) + } + return bin +} + +func TestPipeHeapDumpThroughRedact_ErrorDeletesPartial(t *testing.T) { + tmp := t.TempDir() + src := filepath.Join(tmp, "dump.hprof") + if err := os.WriteFile(src, []byte("FAKE"), 0o600); err != nil { + t.Fatal(err) + } + // Create a partial output file to simulate hprof-redact having started writing + partial := filepath.Join(tmp, "dump-redacted.hprof") + if err := os.WriteFile(partial, []byte("PARTIAL"), 0o600); err != nil { + t.Fatal(err) + } + + failBin := makeFakeBin(t, 1) + _, err := pipeHeapDumpThroughRedact(failBin, src, "lean", false, false) + if err == nil { + t.Fatal("expected error from failing redact binary, got nil") + } + + if _, statErr := os.Stat(partial); !os.IsNotExist(statErr) { + t.Error("partial redacted file should have been deleted on error, but still exists") + } + // source must still be present (we only delete source on success) + if _, statErr := os.Stat(src); statErr != nil { + t.Errorf("source file unexpectedly removed on error: %v", statErr) + } +} + +func TestPipeHeapDumpThroughRedact_KeepOnError(t *testing.T) { + tmp := t.TempDir() + src := filepath.Join(tmp, "dump.hprof") + if err := os.WriteFile(src, []byte("FAKE"), 0o600); err != nil { + t.Fatal(err) + } + partial := filepath.Join(tmp, "dump-redacted.hprof") + if err := os.WriteFile(partial, []byte("PARTIAL"), 0o600); err != nil { + t.Fatal(err) + } + + failBin := makeFakeBin(t, 1) + _, err := pipeHeapDumpThroughRedact(failBin, src, "lean", false, true) + if err == nil { + t.Fatal("expected error from failing redact binary, got nil") + } + + if _, statErr := os.Stat(partial); statErr != nil { + t.Error("partial file should have been kept with keepOnError=true, but is gone") + } +} + +func TestPipeHeapDumpThroughRedact_HappyPath(t *testing.T) { + tmp := t.TempDir() + src := filepath.Join(tmp, "dump.hprof") + if err := os.WriteFile(src, []byte("HEAP"), 0o600); err != nil { + t.Fatal(err) + } + + // Fake binary: copy input to output and exit 0 + binDir := t.TempDir() + bin := filepath.Join(binDir, "fake-redact") + script := "#!/bin/sh\ncp \"$1\" \"$2\"\n" + if err := os.WriteFile(bin, []byte(script), 0o755); err != nil { //nolint:gosec // test binary must be executable + t.Fatal(err) + } + + out, err := pipeHeapDumpThroughRedact(bin, src, "lean", false, false) + if err != nil { + t.Fatalf("expected success, got: %v", err) + } + + want := filepath.Join(tmp, "dump-redacted.hprof") + if out != want { + t.Errorf("output path: want %q, got %q", want, out) + } + if _, statErr := os.Stat(out); statErr != nil { + t.Errorf("output file missing: %v", statErr) + } + // source must be deleted on success + if _, statErr := os.Stat(src); !os.IsNotExist(statErr) { + t.Error("source file should have been deleted after successful redaction") + } +} From fbe08ec99ca97db8b4fe34a78d5dd0f6014677ee Mon Sep 17 00:00:00 2001 From: Johannes Bechberger Date: Mon, 21 Sep 2026 10:24:32 +0200 Subject: [PATCH 29/39] Stream heap dumps through hprof-redact --- .github/workflows/build-and-snapshot.yml | 4 +- .gitignore | 51 ++++++++- .tool.yaml | 32 +++++- CHANGELOG.md | 4 + Makefile | 2 +- README.md | 68 +++++++++--- cf_cli_java_plugin.go | 101 +++++++++++------ open.go | 119 +++++++++++++------- open_test.go | 131 ++++++++++++++++++++++- redact.go | 42 ++++---- redact_test.go | 100 +++++++++++------ utils/cfutils.go | 45 +++++++- 12 files changed, 546 insertions(+), 153 deletions(-) diff --git a/.github/workflows/build-and-snapshot.yml b/.github/workflows/build-and-snapshot.yml index 3c8059d..51027ff 100644 --- a/.github/workflows/build-and-snapshot.yml +++ b/.github/workflows/build-and-snapshot.yml @@ -231,9 +231,9 @@ jobs: ```sh # on Mac arm64 cf install-plugin https://github.com/SAP/cf-cli-java-plugin/releases/download/snapshot/cf-cli-java-plugin-macos-arm64 - # on Windows x86 + # on Windows amd64 cf install-plugin https://github.com/SAP/cf-cli-java-plugin/releases/download/snapshot/cf-cli-java-plugin-windows-amd64 - # on Linux x86 + # on Linux amd64 cf install-plugin https://github.com/SAP/cf-cli-java-plugin/releases/download/snapshot/cf-cli-java-plugin-linux-amd64 ``` diff --git a/.gitignore b/.gitignore index 76b4cfa..59af7df 100644 --- a/.gitignore +++ b/.gitignore @@ -26,7 +26,7 @@ _testmain.go # Tools counterfeiter -# Built binaries +# Build output directories build/ pkg/ @@ -49,11 +49,54 @@ test/snapshots/ # Heap dump files *.hprof -# Build artifacts +# Embedded/downloaded build artifacts dist -# go -pkg +# Local project binaries +requires + +# OS/editor/local tooling +.DS_Store +.claude/ +.playwright-mcp/ +.test_success_cache.json +*.log +*.tmp +*.bak +*~ + +# Python local caches / coverage +.coverage +htmlcov/ +.mypy_cache/ +.ruff_cache/ + +# Local investigation / scratch artifacts +BUG*.md +FIX*.md +*_REPORT.md +*analysis*.py +append_*.py +discover*.py +investigate*.py +*_test_output.txt +*_results.txt +*.out +out.zip + +# Local runtime artifacts +sapmachine21-status/ +sapmachine21-status.zip +sapmachine21-heapdump-*.hprof.gz +sapmachine21-heapdump-*-redacted.hprof.gz + +# Local ad-hoc test helpers/artifacts +test/doc_bugs_finder.py +test/sapmachine21-status.zip +test/test_bugs.py +test_bugs.sh +test_edge_cases.py +test_fixes.py # Internal planning docs (superpowers skill artifacts) docs/superpowers/ diff --git a/.tool.yaml b/.tool.yaml index d5d4b72..a52323e 100644 --- a/.tool.yaml +++ b/.tool.yaml @@ -2,7 +2,8 @@ tag: ready github_url: https://github.com/SAP/cf-cli-java-plugin tagline: Cloud Foundry CLI plugin to troubleshoot Java apps running on CF without SSH. Trigger heap dumps, thread dumps, and async-profiler or JFR recordings from the cf command line, with results - streamed back to your machine. Also embeds jstall for full JVM inspection via `cf java jstall`. + streamed back to your machine. Heap dumps can be redacted during download, kept compressed, or + opened directly in hprof-analyzer. Also embeds jstall for full JVM inspection via `cf java jstall`. tagline_short: Trigger heap dumps, thread dumps, and profiles from the CF CLI — no SSH needed. when_to_use: - You run Java apps on Cloud Foundry and need heap dumps, thread dumps, or CPU profiles @@ -24,6 +25,7 @@ install: # Pick the binary for your platform: cf install-plugin https://github.com/SAP/cf-cli-java-plugin/releases/latest/download/cf-cli-java-plugin-macos-arm64 # linux-amd64 / linux-arm64 / windows-amd64 / windows-arm64 also available + # macOS requires Apple Silicon; macOS Intel (darwin/amd64) is not supported - label: CF Community lang: bash code: | @@ -34,8 +36,19 @@ usage: lang: bash code: | cf java heap-dump my-app + cf java heap-dump my-app --redact --open + cf java heap-dump my-app --redact --compress cf java thread-dump my-app cf java jstall my-app +features: +- title: Stream heap dumps directly to your machine + body: Capture heap dumps and thread dumps from Cloud Foundry apps without manual SSH sessions. +- title: Redact sensitive heap-dump data during download + body: Use `--redact` or `--redact-complete` to zero sensitive primitive data while the dump is being streamed, so an unredacted heap dump is never written locally. +- title: Keep dumps compressed or open them immediately + body: Use `--compress` to save `.hprof.gz` files directly, or `--open` / `--open-url` to inspect the heap dump in hprof-analyzer right after download. +- title: Run bundled jstall diagnostics + body: Inspect deadlocks, hot threads, flame graphs, and more via `cf java jstall` without separately installing jstall. how_to: - title: My CF app is not responding — find what it is stuck on body: | @@ -201,6 +214,23 @@ how_to: ``` The unredacted dump is never written to disk — redaction happens in-memory during download. +- title: Capture a safer heap dump for sharing or browser analysis + body: | + If you need to inspect a heap dump locally or share it with others, combine redaction, + compression, and browser opening as needed: + ```bash + # Redact sensitive values and keep the dump compressed: + cf java heap-dump $APP_NAME --redact --compress + + # Redact and open immediately in hprof-analyzer: + cf java heap-dump $APP_NAME --redact --open + + # Use complete redaction for maximum privacy: + cf java heap-dump $APP_NAME --redact-complete --compress + ``` + Use `--redact-keep-on-error` only if you explicitly want to keep a partially written redacted + file when local redaction fails. + - title: Open a heap dump in hprof-analyzer body: | After downloading, the plugin can spin up a temporary local server and open diff --git a/CHANGELOG.md b/CHANGELOG.md index fb7b6fc..7152d3d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ## [Unreleased] +### Changed + +- macOS plugin support now requires Apple Silicon. macOS Intel (`darwin/amd64`) is not supported. + ### Added - Bundle [jstall](https://github.com/parttimenerd/jstall) (jstall-minimal.jar) for one-shot JVM inspection via diff --git a/Makefile b/Makefile index 14eb27d..35376fb 100644 --- a/Makefile +++ b/Makefile @@ -38,7 +38,7 @@ endif # Downloaded at compile time from hprof-analyzer GitHub releases. # Uses musl-static Linux builds so the binary runs in CF containers without # glibc version constraints. -HPROF_REDACT_BASE = https://github.com/parttimenerd/hprof-analyzer/releases/latest/download +HPROF_REDACT_BASE = https://github.com/parttimenerd/hprof-analyzer/releases/download/nightly dist/hprof-redact-linux-amd64: mkdir -p dist diff --git a/README.md b/README.md index 84ce1f0..1f2b96f 100644 --- a/README.md +++ b/README.md @@ -15,11 +15,14 @@ Currently, it allows you to: ([SapMachine](https://sapmachine.io) only) profiles - Run [jstall](https://github.com/parttimenerd/jstall) for one-shot JVM inspection (deadlock detection, hot threads, dependency graphs, and more): bundled directly in the plugin, requires Java 17+ locally -- Redact heap dumps before saving to remove sensitive data (`--redact`, `--redact-complete`) +- Redact heap dumps before saving to remove sensitive data (`--redact`, `--redact-complete`) using the bundled + [`hprof-redact`](https://github.com/parttimenerd/hprof-analyzer) binary from the + [`hprof-analyzer`](https://github.com/parttimenerd/hprof-analyzer) project - Automatically compress heap dump transfers over SSH on JDK 17+ containers; use `--compress` to keep the local file as `.hprof.gz` -- Open heap dumps directly in the [hprof-analyzer](https://parttimenerd.github.io/hprof-analyzer) web app - after downloading (`--open`) +- Open heap dumps directly in the hosted + [`hprof-analyzer`](https://parttimenerd.github.io/hprof-analyzer) web app after downloading (`--open`) or point the + plugin at another `hprof-analyzer` instance via `--open-url` ## Installation @@ -44,16 +47,18 @@ Download the latest release from [GitHub](https://github.com/SAP/cf-cli-java-plu To install a new version of the plugin, run the following: ```sh -# on Mac arm64 +# on Mac arm64 (Apple Silicon only) cf install-plugin https://github.com/SAP/cf-cli-java-plugin/releases/latest/download/cf-cli-java-plugin-macos-arm64 -# on Windows x64 +# on Windows amd64 cf install-plugin https://github.com/SAP/cf-cli-java-plugin/releases/latest/download/cf-cli-java-plugin-windows-amd64 -# on Linux x64 +# on Linux amd64 cf install-plugin https://github.com/SAP/cf-cli-java-plugin/releases/latest/download/cf-cli-java-plugin-linux-amd64 # on Linux arm64 cf install-plugin https://github.com/SAP/cf-cli-java-plugin/releases/latest/download/cf-cli-java-plugin-linux-arm64 ``` +macOS plugin binaries currently require Apple Silicon; macOS Intel (`darwin/amd64`) is not supported. + You can verify that the plugin is successfully installed by looking for `java` in the output of `cf plugins`. ### Manual Installation of Snapshot Release @@ -64,16 +69,18 @@ This is intended for experimentation and might fail. To install a new version of the plugin, run the following: ```sh -# on Mac arm64 +# on Mac arm64 (Apple Silicon only) cf install-plugin https://github.com/SAP/cf-cli-java-plugin/releases/download/snapshot/cf-cli-java-plugin-macos-arm64 -# on Windows x64 +# on Windows amd64 cf install-plugin https://github.com/SAP/cf-cli-java-plugin/releases/download/snapshot/cf-cli-java-plugin-windows-amd64 -# on Linux x64 +# on Linux amd64 cf install-plugin https://github.com/SAP/cf-cli-java-plugin/releases/download/snapshot/cf-cli-java-plugin-linux-amd64 # on Linux arm64 cf install-plugin https://github.com/SAP/cf-cli-java-plugin/releases/download/snapshot/cf-cli-java-plugin-linux-arm64 ``` +macOS snapshot binaries currently require Apple Silicon; macOS Intel (`darwin/amd64`) is not supported. + ## Common Tasks ### My CF app is not responding — find what it is stuck on @@ -224,10 +231,15 @@ cf java heap-dump $APP_NAME --open --redact --compress cf java heap-dump $APP_NAME --open-url http://localhost:8080 ``` +The browser integration uses the [`hprof-analyzer`](https://github.com/parttimenerd/hprof-analyzer) project. By +default, `--open` launches the hosted web app at . Use `--open-url` if +you run your own local or internal `hprof-analyzer` deployment. + > **macOS note:** On macOS with the Application Firewall enabled, a dialog will appear asking > *"Do you want the application 'cf-cli-java-plugin' to accept incoming network connections?"* > Click **Allow** — the plugin binds a temporary local server on `127.0.0.1` to serve the file -> to the browser. The server shuts down automatically after the browser fetches the file once. +> to the browser. The server serves only the exact one-time heap-dump URL generated for that download, +> rejects alternate paths or query parameters, and shuts down automatically after one successful fetch. Getting a thread dump: @@ -359,6 +371,22 @@ cf java thread-dump [my_app] -i [my_instance_index] > thread-dump.txt The `--keep` flag is not applicable to commands that stream output directly (e.g., `thread-dump`). +Heap dumps support additional local post-processing and analysis options: + +- `--redact`: lean redaction mode; streams the heap dump through `hprof-redact` and zeros primitive arrays such as + `byte[]`, `char[]`, and similar bulk buffers +- `--redact-complete`: complete redaction mode; streams the heap dump through `hprof-redact` and zeros primitive arrays + and individual primitive fields +- `--redact-keep-on-error`: keeps a partially written redacted output file if local redaction fails; otherwise failed + redaction leaves no local heap dump behind +- `--compress`: keeps the local output as `.hprof.gz` instead of transparently decompressing it +- `--open`: starts a temporary local HTTP server on `127.0.0.1`, serves the downloaded heap dump once, and opens + [`hprof-analyzer`](https://parttimenerd.github.io/hprof-analyzer) automatically in your browser +- `--open-url `: same as `--open`, but targets a custom hosted or self-managed `hprof-analyzer` instance + +These features can be combined, for example: `cf java heap-dump APP --redact --compress --open`. +`--open` requires a local file and therefore cannot be used with `--no-download`. + ### Heap Dump Privacy Heap dumps contain the full in-memory state of a JVM, including strings, byte arrays, and field values, which can @@ -373,11 +401,14 @@ use `--redact` or `--redact-complete` to zero out sensitive values. Both modes preserve the full object graph (class names, references, instance counts), so the dump remains useful for memory analysis. The two flags are mutually exclusive. -The redacted file is saved locally with a `-redacted` suffix; the original unredacted file is deleted automatically. +The redacted file is saved to the requested local heap-dump path with no extra suffix. When redaction is enabled, the +heap dump is streamed directly into `hprof-redact` exactly as downloaded, including gzip-compressed `.hprof.gz` +streams, so the unredacted dump is never written to local disk. Use `--redact --compress` to also compress the output (produces a `.hprof.gz`). -Redaction runs locally via the bundled [hprof-redact](https://github.com/parttimenerd/hprof-analyzer) binary after -the dump is downloaded. Supported platforms: Linux (x86_64, arm64), macOS (Apple Silicon), Windows (x86_64, arm64). +Redaction runs locally via the bundled [hprof-redact](https://github.com/parttimenerd/hprof-analyzer) binary while the +dump is being downloaded. The binary is embedded from the +[`hprof-analyzer`](https://github.com/parttimenerd/hprof-analyzer) project, so no separate installation is required. ### Compressed Transfer @@ -391,6 +422,17 @@ Without `--compress`, the plugin still uses `gz=1` automatically when the remote compressed but the local file is transparently decompressed to a plain `.hprof`. This is the default behaviour starting from JDK 17 and costs nothing from the user's perspective. +### Opening a Heap Dump in hprof-analyzer + +Use `--open` to inspect the downloaded heap dump immediately in +[`hprof-analyzer`](https://github.com/parttimenerd/hprof-analyzer), either via the hosted instance at + or via your own deployment with `--open-url`. + +For safety, the plugin does **not** expose an arbitrary local directory. Instead, it starts a temporary local HTTP +server bound to `127.0.0.1`, serves only the exact generated heap-dump for that one download, rejects alternate +paths and query parameters, and shuts the server down automatically after one successful browser fetch or after a +timeout if the browser never connects. + ## Limitations Some commands depend on writable filesystem space inside the application container. In particular, `cf java heap-dump`, diff --git a/cf_cli_java_plugin.go b/cf_cli_java_plugin.go index 9cf7077..ca6a160 100644 --- a/cf_cli_java_plugin.go +++ b/cf_cli_java_plugin.go @@ -11,6 +11,7 @@ package main import ( "errors" "fmt" + "io" "os" "os/exec" "path/filepath" @@ -1305,46 +1306,58 @@ func (c *JavaPlugin) execute(_ plugin.CliConnection, args []string) (string, err localFileExt = extHprofGz case !remoteIsGz && options.Compress: fmt.Fprintf(os.Stderr, "Warning: remote jmap does not support gz compression (JDK 17+ required); downloading uncompressed\n") - case remoteIsGz: - fmt.Println("Note: remote jmap used gz compression; decompressing during transfer...") } } localFileFullPath := localDir + "/" + applicationName + "-" + command.FileNamePart + "-" + utils.GenerateUUID() + localFileExt c.logVerbosef("Downloading file to: %s", localFileFullPath) - if command.Name == cmdHeapDump && remoteIsGz && localFileExt == extHprof { - // Transparent decompression: stream gz from remote, write plain .hprof locally - err = utils.CopyOverCatGunzip(cfSSHArguments, fileName, localFileFullPath) - } else { - err = utils.CopyOverCat(cfSSHArguments, fileName, localFileFullPath) + redactingHeapDump := command.Name == cmdHeapDump && (options.Redact || options.RedactComplete) + if command.Name == cmdHeapDump && remoteIsGz && localFileExt == extHprof && !redactingHeapDump { + fmt.Println("Note: remote jmap used gz compression; decompressing during transfer...") } - if err == nil { - c.logVerbosef("File download completed successfully") - fmt.Println(utils.ToSentenceCase(command.FileLabel) + " file saved to: " + localFileFullPath) + if redactingHeapDump { + mode := "lean" + if options.RedactComplete { + mode = "complete" + } + redactBin, rerr := ensureHprofRedact() + if rerr != nil { + return "", fmt.Errorf("hprof-redact unavailable: %w", rerr) + } - finalLocalPath := localFileFullPath - if command.Name == cmdHeapDump && (options.Redact || options.RedactComplete) { - mode := "lean" - if options.RedactComplete { - mode = "complete" - } - redactBin, rerr := ensureHprofRedact() - if rerr != nil { - return "", fmt.Errorf("hprof-redact unavailable: %w", rerr) + var reader io.ReadCloser + var waitRemote func() error + reader, waitRemote, err = utils.StreamOverCat(cfSSHArguments, fileName) + if err != nil { + return "", err + } + + finalPath, rerr := pipeHeapDumpThroughRedact(redactBin, reader, localFileFullPath, mode, options.RedactKeepOnError) + closeErr := reader.Close() + waitErr := waitRemote() + if rerr != nil { + if closeErr != nil { + c.logVerbosef("warning: closing redaction input stream failed: %v", closeErr) } - localIsGz := strings.HasSuffix(localFileFullPath, extHprofGz) - finalPath, rerr := pipeHeapDumpThroughRedact(redactBin, localFileFullPath, mode, options.Compress || localIsGz, options.RedactKeepOnError) - if rerr != nil { - return "", fmt.Errorf("redaction failed: %w", rerr) + if waitErr != nil { + c.logVerbosef("warning: remote stream wait after redaction failure failed: %v", waitErr) } - fmt.Println("Redacted heap dump saved to: " + finalPath) - finalLocalPath = finalPath + return "", fmt.Errorf("redaction failed: %w", rerr) + } + if closeErr != nil { + return "", fmt.Errorf("redaction input stream close failed: %w", closeErr) } + if waitErr != nil { + return "", fmt.Errorf("download failed while streaming redaction input: %w", waitErr) + } + + c.logVerbosef("Redacted heap dump stream completed successfully") + fmt.Println("Redacted heap dump saved to: " + finalPath) if command.Name == cmdHeapDump && options.Open { - port, urlFile, done, serveErr := serveFileOnce(finalLocalPath, 10*time.Minute) + port, urlFile, done, serveErr := serveFileOnce(finalPath, 10*time.Minute) if serveErr != nil { return "", fmt.Errorf("could not start local file server: %w", serveErr) } @@ -1354,12 +1367,36 @@ func (c *JavaPlugin) execute(_ plugin.CliConnection, args []string) (string, err <-done } } else { - c.logVerbosef("File download failed: %v", err) - fmt.Fprintf(os.Stderr, "The %s was created successfully in the container at: %s\n", command.FileLabel, fileName) - fmt.Fprintf(os.Stderr, "However, downloading to local failed: %v\n", err) - fmt.Fprintf(os.Stderr, "The remote file is still available. Retry with:\n") - fmt.Fprintf(os.Stderr, " cf ssh %s -c 'cat %s' > %s\n", applicationName, fileName, localFileFullPath) - return "", fmt.Errorf("download failed (remote file intact): %w", err) + if command.Name == cmdHeapDump && remoteIsGz && localFileExt == extHprof { + // Transparent decompression: stream gz from remote, write plain .hprof locally + err = utils.CopyOverCatGunzip(cfSSHArguments, fileName, localFileFullPath) + } else { + err = utils.CopyOverCat(cfSSHArguments, fileName, localFileFullPath) + } + + if err == nil { + c.logVerbosef("File download completed successfully") + fmt.Println(utils.ToSentenceCase(command.FileLabel) + " file saved to: " + localFileFullPath) + + finalLocalPath := localFileFullPath + if command.Name == cmdHeapDump && options.Open { + port, urlFile, done, serveErr := serveFileOnce(finalLocalPath, 10*time.Minute) + if serveErr != nil { + return "", fmt.Errorf("could not start local file server: %w", serveErr) + } + openURL := buildOpenURL(options.OpenURL, port, urlFile) + fmt.Printf("Opening heap dump in browser: %s\n", openURL) + openBrowser(openURL) + <-done + } + } else { + c.logVerbosef("File download failed: %v", err) + fmt.Fprintf(os.Stderr, "The %s was created successfully in the container at: %s\n", command.FileLabel, fileName) + fmt.Fprintf(os.Stderr, "However, downloading to local failed: %v\n", err) + fmt.Fprintf(os.Stderr, "The remote file is still available. Retry with:\n") + fmt.Fprintf(os.Stderr, " cf ssh %s -c 'cat %s' > %s\n", applicationName, fileName, localFileFullPath) + return "", fmt.Errorf("download failed (remote file intact): %w", err) + } } if !keepAfterDownload { diff --git a/open.go b/open.go index c9cf4de..0354b24 100644 --- a/open.go +++ b/open.go @@ -14,6 +14,7 @@ import ( "io" "net" "net/http" + "net/url" "os" "os/exec" "runtime" @@ -29,8 +30,12 @@ import ( // closes when the first GET request completes or timeout elapses. // timeout 0 means no timeout. func serveFileOnce(path string, timeout time.Duration) (port int, urlFile string, done <-chan struct{}, err error) { - if _, err = os.Stat(path); err != nil { - return 0, "", nil, fmt.Errorf("file not found: %w", err) + info, statErr := os.Stat(path) + if statErr != nil { + return 0, "", nil, fmt.Errorf("file not found: %w", statErr) + } + if !info.Mode().IsRegular() { + return 0, "", nil, fmt.Errorf("path is not a regular file: %s", path) } // Build a random token + preserve only the file extension (.hprof or .hprof.gz). @@ -44,6 +49,8 @@ func serveFileOnce(path string, timeout time.Duration) (port int, urlFile string ext = extHprofGz } urlFile = token + ext + exactPath := "/" + urlFile + exactEscapedPath := (&url.URL{Path: exactPath}).EscapedPath() ln, err := net.Listen("tcp", "127.0.0.1:0") if err != nil { @@ -53,48 +60,53 @@ func serveFileOnce(path string, timeout time.Duration) (port int, urlFile string doneCh := make(chan struct{}) var shutdownOnce sync.Once - mux := http.NewServeMux() - srv := &http.Server{ - Handler: mux, + var srv *http.Server + srv = &http.Server{ + Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != exactPath || r.URL.EscapedPath() != exactEscapedPath || r.URL.RawQuery != "" { + http.NotFound(w, r) + return + } + w.Header().Set("Access-Control-Allow-Origin", "*") + // Answer CORS preflight without serving the file or triggering shutdown. + if r.Method == http.MethodOptions { + w.Header().Set("Access-Control-Allow-Methods", "GET, OPTIONS") + w.WriteHeader(http.StatusNoContent) + return + } + if r.Method != http.MethodGet { + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + return + } + f, ferr := os.Open(path) //nolint:gosec // path comes from plugin internals, not user input + if ferr != nil { + http.Error(w, "file unavailable", http.StatusInternalServerError) + return + } + defer func() { _ = f.Close() }() + w.Header().Set("Content-Type", "application/octet-stream") + w.WriteHeader(http.StatusOK) + if _, copyErr := io.Copy(w, f); copyErr != nil { + fmt.Fprintf(os.Stderr, "warning: failed while serving heap dump: %v\n", copyErr) + return + } + // Use a fresh context: r.Context() is canceled when the handler returns, + // but Shutdown must outlive the request. + // sync.Once ensures concurrent GETs can't double-close doneCh (panic). + shutdownCtx, shutdownCancel := context.WithTimeout(context.Background(), 5*time.Second) //nolint:contextcheck + go func(ctx context.Context, cancel context.CancelFunc) { //nolint:contextcheck + defer cancel() + shutdownOnce.Do(func() { + close(doneCh) + _ = srv.Shutdown(ctx) + }) + }(shutdownCtx, shutdownCancel) + }), ReadHeaderTimeout: 10 * time.Second, + IdleTimeout: 30 * time.Second, WriteTimeout: 30 * time.Second, } - // Register only the exact random path — any other request gets 404. - mux.HandleFunc("/"+urlFile, func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Access-Control-Allow-Origin", "*") - // Answer CORS preflight without serving the file or triggering shutdown. - if r.Method == http.MethodOptions { - w.Header().Set("Access-Control-Allow-Methods", "GET, OPTIONS") - w.WriteHeader(http.StatusNoContent) - return - } - if r.Method != http.MethodGet { - http.Error(w, "method not allowed", http.StatusMethodNotAllowed) - return - } - f, ferr := os.Open(path) //nolint:gosec // path comes from plugin internals, not user input - if ferr != nil { - http.Error(w, "file unavailable", http.StatusInternalServerError) - return - } - defer func() { _ = f.Close() }() - w.Header().Set("Content-Type", "application/octet-stream") - w.WriteHeader(http.StatusOK) - _, _ = io.Copy(w, f) - // Use a fresh context: r.Context() is canceled when the handler returns, - // but Shutdown must outlive the request. - // sync.Once ensures concurrent GETs can't double-close doneCh (panic). - shutdownCtx, shutdownCancel := context.WithTimeout(context.Background(), 5*time.Second) //nolint:contextcheck - go func(ctx context.Context, cancel context.CancelFunc) { //nolint:contextcheck - defer cancel() - shutdownOnce.Do(func() { - close(doneCh) - _ = srv.Shutdown(ctx) - }) - }(shutdownCtx, shutdownCancel) - }) - go func() { _ = srv.Serve(ln) }() if timeout > 0 { @@ -122,12 +134,37 @@ func serveFileOnce(path string, timeout time.Duration) (port int, urlFile string // base is the analyzer base URL (trailing slash optional). // Use port=0 to produce a PORT placeholder (for dry-run output). func buildOpenURL(base string, port int, filename string) string { + hadTrailingSlash := strings.HasSuffix(base, "/") base = strings.TrimRight(base, "/") portStr := fmt.Sprintf("%d", port) if port == 0 { portStr = "PORT" } - return fmt.Sprintf("%s/?file=http://localhost:%s/%s", base, portStr, filename) + + parsedBase, err := url.Parse(base) + if err != nil || parsedBase.Scheme == "" || parsedBase.Host == "" { + return fmt.Sprintf("%s/?file=%s", base, url.QueryEscape(fmt.Sprintf("http://localhost:%s/%s", portStr, filename))) + } + + fileURL := &url.URL{ + Scheme: "http", + Host: "localhost:" + portStr, + Path: "/" + filename, + } + query := parsedBase.Query() + query.Set("file", fileURL.String()) + parsedBase.RawQuery = query.Encode() + if hadTrailingSlash && !strings.HasSuffix(parsedBase.Path, "/") { + parsedBase.Path += "/" + } + if parsedBase.RawPath == "" { + parsedBase.RawPath = parsedBase.Path + } + if !hadTrailingSlash && parsedBase.RawQuery != "" && !strings.HasSuffix(parsedBase.Path, "/") && parsedBase.RawPath == parsedBase.Path { + parsedBase.Path += "/" + parsedBase.RawPath += "/" + } + return parsedBase.String() } // openBrowser opens url in the system default browser. diff --git a/open_test.go b/open_test.go index 5be145c..a8c3772 100644 --- a/open_test.go +++ b/open_test.go @@ -3,7 +3,9 @@ package main import ( "fmt" "io" + "net" "net/http" + "net/url" "os" "path/filepath" "strings" @@ -200,19 +202,19 @@ func TestBuildOpenURL(t *testing.T) { "https://parttimenerd.github.io/hprof-analyzer", 54321, "myapp-heapdump-abc.hprof", - "https://parttimenerd.github.io/hprof-analyzer/?file=http://localhost:54321/myapp-heapdump-abc.hprof", + "https://parttimenerd.github.io/hprof-analyzer/?file=http%3A%2F%2Flocalhost%3A54321%2Fmyapp-heapdump-abc.hprof", }, { "https://parttimenerd.github.io/hprof-analyzer/", 9000, "dump.hprof.gz", - "https://parttimenerd.github.io/hprof-analyzer/?file=http://localhost:9000/dump.hprof.gz", + "https://parttimenerd.github.io/hprof-analyzer/?file=http%3A%2F%2Flocalhost%3A9000%2Fdump.hprof.gz", }, { "https://parttimenerd.github.io/hprof-analyzer", 0, "dump.hprof", - "https://parttimenerd.github.io/hprof-analyzer/?file=http://localhost:PORT/dump.hprof", + "https://parttimenerd.github.io/hprof-analyzer/?file=http%3A%2F%2Flocalhost%3APORT%2Fdump.hprof", }, } for _, tc := range cases { @@ -230,6 +232,79 @@ func TestBuildOpenURL_TrailingSlash(t *testing.T) { } } +func TestBuildOpenURL_EscapesFilenameAndPreservesExistingQuery(t *testing.T) { + got := buildOpenURL("https://example.com/analyzer/?theme=dark", 1234, "dump name+#1.hprof.gz") + want := "https://example.com/analyzer/?file=http%3A%2F%2Flocalhost%3A1234%2Fdump%2520name%2B%25231.hprof.gz&theme=dark" + if got != want { + t.Fatalf("buildOpenURL escaped URL mismatch\nwant: %s\n got: %s", want, got) + } +} + +func TestServeFileOnce_RejectsDirectory(t *testing.T) { + tmp := t.TempDir() + _, _, _, err := serveFileOnce(tmp, time.Second) + if err == nil { + t.Fatal("expected error for directory input, got nil") + } + if !strings.Contains(err.Error(), "not a regular file") { + t.Fatalf("expected regular file error, got: %v", err) + } +} + +func TestServeFileOnce_RejectsPathVariants(t *testing.T) { + tmp := t.TempDir() + p := filepath.Join(tmp, "test.hprof") + if err := os.WriteFile(p, []byte("SECRET"), 0o600); err != nil { + t.Fatal(err) + } + + port, urlFile, done, err := serveFileOnce(p, 30*time.Second) + if err != nil { + t.Fatalf("serveFileOnce: %v", err) + } + + badURLs := []string{ + fmt.Sprintf("http://localhost:%d//%s", port, urlFile), + fmt.Sprintf("http://localhost:%d/%s/", port, urlFile), + fmt.Sprintf("http://localhost:%d/%s%%2f", port, urlFile), + fmt.Sprintf("http://localhost:%d/%s?extra=1", port, url.QueryEscape(urlFile)), + } + + for _, rawURL := range badURLs { + resp, gerr := http.Get(rawURL) //nolint:noctx,gosec + if gerr != nil { + t.Fatalf("GET %s: %v", rawURL, gerr) + } + _ = resp.Body.Close() + if resp.StatusCode != http.StatusNotFound { + t.Fatalf("GET %s: want 404, got %d", rawURL, resp.StatusCode) + } + } + + select { + case <-done: + t.Fatal("done channel closed after non-exact path variant request") + default: + } + + goodURL := fmt.Sprintf("http://localhost:%d/%s", port, urlFile) + resp, err := http.Get(goodURL) //nolint:noctx,gosec + if err != nil { + t.Fatalf("GET %s: %v", goodURL, err) + } + _, _ = io.ReadAll(resp.Body) + _ = resp.Body.Close() + if resp.StatusCode != http.StatusOK { + t.Fatalf("GET %s: want 200, got %d", goodURL, resp.StatusCode) + } + + select { + case <-done: + case <-time.After(2 * time.Second): + t.Fatal("done channel not closed after exact path GET") + } +} + func TestServeFileOnce_ConcurrentGETsNoPanic(t *testing.T) { tmp := t.TempDir() p := filepath.Join(tmp, "test.hprof") @@ -268,3 +343,53 @@ func TestServeFileOnce_ConcurrentGETsNoPanic(t *testing.T) { t.Error("done channel not closed after concurrent GETs") } } + +func TestServeFileOnce_ClientDisconnectDoesNotConsumeSingleServe(t *testing.T) { + tmp := t.TempDir() + p := filepath.Join(tmp, "test.hprof") + content := strings.Repeat("HEAP_CONTENT", 1<<14) + if err := os.WriteFile(p, []byte(content), 0o600); err != nil { + t.Fatal(err) + } + + port, urlFile, done, err := serveFileOnce(p, 30*time.Second) + if err != nil { + t.Fatalf("serveFileOnce: %v", err) + } + + conn, err := net.Dial("tcp", fmt.Sprintf("127.0.0.1:%d", port)) + if err != nil { + t.Fatalf("Dial: %v", err) + } + _, _ = fmt.Fprintf(conn, "GET /%s HTTP/1.1\r\nHost: localhost\r\n\r\n", urlFile) + _ = conn.Close() + + select { + case <-done: + t.Fatal("done channel closed after client disconnected before successful transfer") + case <-time.After(200 * time.Millisecond): + } + + goodURL := fmt.Sprintf("http://localhost:%d/%s", port, urlFile) + resp, err := http.Get(goodURL) //nolint:noctx,gosec + if err != nil { + t.Fatalf("GET %s: %v", goodURL, err) + } + defer func() { _ = resp.Body.Close() }() + body, err := io.ReadAll(resp.Body) + if err != nil { + t.Fatalf("ReadAll: %v", err) + } + if resp.StatusCode != http.StatusOK { + t.Fatalf("GET %s: want 200, got %d", goodURL, resp.StatusCode) + } + if string(body) != content { + t.Fatalf("unexpected body length/content after retry") + } + + select { + case <-done: + case <-time.After(2 * time.Second): + t.Fatal("done channel not closed after successful retry GET") + } +} diff --git a/redact.go b/redact.go index 5d3498a..a0a7837 100644 --- a/redact.go +++ b/redact.go @@ -11,6 +11,7 @@ import ( _ "embed" "encoding/hex" "fmt" + "io" "os" "os/exec" "path/filepath" @@ -90,32 +91,39 @@ func ensureHprofRedact() (string, error) { return binPath, nil } -// pipeHeapDumpThroughRedact runs hprof-redact on localPath, writing the output -// to a new file derived from localPath. On success it deletes the original -// unredacted file and returns the path to the redacted file. +// pipeHeapDumpThroughRedact streams heap dump bytes through hprof-redact using +// stdin (`hprof-redact -`) and writes only the requested output path. // -// mode must be "lean" or "complete". If compress is true the output is written -// as .hprof.gz. -func pipeHeapDumpThroughRedact(redactBin, localPath, mode string, compress bool, keepOnError bool) (string, error) { - base := strings.TrimSuffix(localPath, ".hprof.gz") - base = strings.TrimSuffix(base, ".hprof") - var outputPath string - if compress { - outputPath = base + "-redacted.hprof.gz" +// mode must be "lean" or "complete". outputBasePath must end in .hprof or .hprof.gz. +func pipeHeapDumpThroughRedact(redactBin string, input io.Reader, outputBasePath, mode string, keepOnError bool) (string, error) { + if !strings.HasSuffix(outputBasePath, extHprof) && !strings.HasSuffix(outputBasePath, extHprofGz) { + return "", fmt.Errorf("unsupported heap dump path %q: expected %s or %s suffix", outputBasePath, extHprof, extHprofGz) + } + outputPath := outputBasePath + + if err := os.MkdirAll(filepath.Dir(outputPath), 0o755); err != nil { //nolint:gosec // local output dir for plugin-managed file + return "", fmt.Errorf("cannot create local directory %s: %w", filepath.Dir(outputPath), err) + } + + if _, err := os.OpenFile(outputPath, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o600); err != nil { //nolint:gosec // plugin-constructed path + return "", fmt.Errorf("error creating local file at %s: %w", outputPath, err) } else { - outputPath = base + "-redacted.hprof" + _ = os.Remove(outputPath) } var args []string if mode == "complete" { args = append(args, "--complete") } - args = append(args, localPath, outputPath) + args = append(args, "-", outputPath) cmd := exec.Command(redactBin, args...) //nolint:gosec // redactBin comes from ensureHprofRedact, not user input + cmd.Stdin = input + cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr - if err := cmd.Run(); err != nil { + err := cmd.Run() + if err != nil { if !keepOnError { if rmErr := os.Remove(outputPath); rmErr != nil && !os.IsNotExist(rmErr) { fmt.Fprintf(os.Stderr, "warning: could not remove partial redacted file %s: %v\n", outputPath, rmErr) @@ -124,11 +132,5 @@ func pipeHeapDumpThroughRedact(redactBin, localPath, mode string, compress bool, return "", fmt.Errorf("hprof-redact failed: %w", err) } - // Remove the unredacted source file - if err := os.Remove(localPath); err != nil { - // Non-fatal: redacted file is already written - fmt.Fprintf(os.Stderr, "warning: could not remove unredacted file %s: %v\n", localPath, err) - } - return outputPath, nil } diff --git a/redact_test.go b/redact_test.go index 57d52fe..c6dd217 100644 --- a/redact_test.go +++ b/redact_test.go @@ -1,6 +1,8 @@ package main import ( + "bytes" + "compress/gzip" "fmt" "os" "path/filepath" @@ -21,18 +23,14 @@ func makeFakeBin(t *testing.T, exitCode int) string { func TestPipeHeapDumpThroughRedact_ErrorDeletesPartial(t *testing.T) { tmp := t.TempDir() - src := filepath.Join(tmp, "dump.hprof") - if err := os.WriteFile(src, []byte("FAKE"), 0o600); err != nil { - t.Fatal(err) - } - // Create a partial output file to simulate hprof-redact having started writing - partial := filepath.Join(tmp, "dump-redacted.hprof") + partialBase := filepath.Join(tmp, "dump.hprof") + partial := partialBase if err := os.WriteFile(partial, []byte("PARTIAL"), 0o600); err != nil { t.Fatal(err) } failBin := makeFakeBin(t, 1) - _, err := pipeHeapDumpThroughRedact(failBin, src, "lean", false, false) + _, err := pipeHeapDumpThroughRedact(failBin, bytes.NewBufferString("FAKE"), partialBase, "lean", false) if err == nil { t.Fatal("expected error from failing redact binary, got nil") } @@ -40,25 +38,20 @@ func TestPipeHeapDumpThroughRedact_ErrorDeletesPartial(t *testing.T) { if _, statErr := os.Stat(partial); !os.IsNotExist(statErr) { t.Error("partial redacted file should have been deleted on error, but still exists") } - // source must still be present (we only delete source on success) - if _, statErr := os.Stat(src); statErr != nil { - t.Errorf("source file unexpectedly removed on error: %v", statErr) - } } func TestPipeHeapDumpThroughRedact_KeepOnError(t *testing.T) { tmp := t.TempDir() - src := filepath.Join(tmp, "dump.hprof") - if err := os.WriteFile(src, []byte("FAKE"), 0o600); err != nil { - t.Fatal(err) - } - partial := filepath.Join(tmp, "dump-redacted.hprof") - if err := os.WriteFile(partial, []byte("PARTIAL"), 0o600); err != nil { + base := filepath.Join(tmp, "dump.hprof") + partial := base + + binDir := t.TempDir() + failBin := filepath.Join(binDir, "fake-redact") + script := "#!/bin/sh\necho PARTIAL > \"$2\"\nexit 1\n" + if err := os.WriteFile(failBin, []byte(script), 0o755); err != nil { //nolint:gosec // test binary must be executable t.Fatal(err) } - - failBin := makeFakeBin(t, 1) - _, err := pipeHeapDumpThroughRedact(failBin, src, "lean", false, true) + _, err := pipeHeapDumpThroughRedact(failBin, bytes.NewBufferString("FAKE"), base, "lean", true) if err == nil { t.Fatal("expected error from failing redact binary, got nil") } @@ -70,33 +63,78 @@ func TestPipeHeapDumpThroughRedact_KeepOnError(t *testing.T) { func TestPipeHeapDumpThroughRedact_HappyPath(t *testing.T) { tmp := t.TempDir() - src := filepath.Join(tmp, "dump.hprof") - if err := os.WriteFile(src, []byte("HEAP"), 0o600); err != nil { - t.Fatal(err) - } + base := filepath.Join(tmp, "dump.hprof") - // Fake binary: copy input to output and exit 0 + // Fake binary: copy stdin to output and exit 0 binDir := t.TempDir() bin := filepath.Join(binDir, "fake-redact") - script := "#!/bin/sh\ncp \"$1\" \"$2\"\n" + script := "#!/bin/sh\ncat - > \"$2\"\n" if err := os.WriteFile(bin, []byte(script), 0o755); err != nil { //nolint:gosec // test binary must be executable t.Fatal(err) } - out, err := pipeHeapDumpThroughRedact(bin, src, "lean", false, false) + out, err := pipeHeapDumpThroughRedact(bin, bytes.NewBufferString("HEAP"), base, "lean", false) if err != nil { t.Fatalf("expected success, got: %v", err) } - want := filepath.Join(tmp, "dump-redacted.hprof") + want := filepath.Join(tmp, "dump.hprof") if out != want { t.Errorf("output path: want %q, got %q", want, out) } if _, statErr := os.Stat(out); statErr != nil { t.Errorf("output file missing: %v", statErr) } - // source must be deleted on success - if _, statErr := os.Stat(src); !os.IsNotExist(statErr) { - t.Error("source file should have been deleted after successful redaction") + data, readErr := os.ReadFile(out) //nolint:gosec // test reads file path produced by helper under test + if readErr != nil { + t.Fatalf("read output: %v", readErr) + } + if string(data) != "HEAP" { + t.Fatalf("unexpected output content: %q", string(data)) + } +} + +func TestPipeHeapDumpThroughRedact_PassesCompressedInputUnchanged(t *testing.T) { + tmp := t.TempDir() + base := filepath.Join(tmp, "dump.hprof") + + var compressed bytes.Buffer + gz := gzip.NewWriter(&compressed) + if _, err := gz.Write([]byte("HEAP")); err != nil { + t.Fatalf("gzip write: %v", err) + } + if err := gz.Close(); err != nil { + t.Fatalf("gzip close: %v", err) + } + + binDir := t.TempDir() + bin := filepath.Join(binDir, "fake-redact") + script := "#!/bin/sh\ncat - > \"$2\"\n" + if err := os.WriteFile(bin, []byte(script), 0o755); err != nil { //nolint:gosec // test binary must be executable + t.Fatal(err) + } + + out, err := pipeHeapDumpThroughRedact(bin, bytes.NewReader(compressed.Bytes()), base, "lean", false) + if err != nil { + t.Fatalf("expected success, got: %v", err) + } + + data, err := os.ReadFile(out) //nolint:gosec // test reads file path produced by helper under test + if err != nil { + t.Fatalf("read output: %v", err) + } + if !bytes.Equal(data, compressed.Bytes()) { + t.Fatal("compressed input was modified before reaching hprof-redact") + } +} + +func TestPipeHeapDumpThroughRedact_RejectsUnexpectedExtension(t *testing.T) { + tmp := t.TempDir() + base := filepath.Join(tmp, "dump.bin") + + bin := makeFakeBin(t, 0) + _, err := pipeHeapDumpThroughRedact(bin, bytes.NewBufferString("HEAP"), base, "lean", false) + if err == nil { + t.Fatal("expected unsupported extension error, got nil") } } diff --git a/utils/cfutils.go b/utils/cfutils.go index 171f0b8..cfddef1 100644 --- a/utils/cfutils.go +++ b/utils/cfutils.go @@ -19,6 +19,14 @@ import ( "github.com/lithammer/fuzzysearch/fuzzy" ) +func shellSingleQuote(s string) string { + return "'" + strings.ReplaceAll(s, "'", `"'"'`) + "'" +} + +func remoteCatCommand(src string) string { + return "exec cat -- " + shellSingleQuote(src) +} + // Version represents a semantic version with major, minor, and build numbers. type Version struct { Major int @@ -211,7 +219,7 @@ func GetAvailablePath(data string, userpath string) (string, error) { return "/tmp", nil } -// CopyOverCat copies a remote file to a local destination using the cf ssh command and cat. +// CopyOverCat copies a remote file to a local destination using cf ssh. func CopyOverCat(args []string, src string, dest string) error { // Ensure parent directory exists if dir := filepath.Dir(dest); dir != "" && dir != "." { @@ -221,7 +229,7 @@ func CopyOverCat(args []string, src string, dest string) error { } f, err := os.OpenFile(dest, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0o600) //nolint:gosec // dest is a plugin-constructed output path, not user-supplied file inclusion if err != nil { - return errors.New("Error creating local file at " + dest + ". Please check that you are allowed to create files at the given local path.") + return errors.New("Error creating local file at " + dest + ". Please check that you are allowed to create files at the given local path.") } defer func() { if closeErr := f.Close(); closeErr != nil { @@ -230,7 +238,7 @@ func CopyOverCat(args []string, src string, dest string) error { } }() - args = append(args, "cat \""+src+"\"") + args = append(args, remoteCatCommand(src)) cat := exec.Command("cf", args...) cat.Stdout = f @@ -242,12 +250,39 @@ func CopyOverCat(args []string, src string, dest string) error { err = cat.Wait() if err != nil { - return errors.New("error occurred while waiting for the copying complete") + return errors.New("error occurred while waiting for the file copy to complete") } return nil } +// StreamOverCat starts a remote `cat` over cf ssh and returns a reader for the +// remote file plus a wait function that must be called after the reader is fully +// consumed. +func StreamOverCat(args []string, src string) (io.ReadCloser, func() error, error) { + pr, pw := io.Pipe() + catArgs := append(args, remoteCatCommand(src)) //nolint:gocritic // intentional new slice + cat := exec.Command("cf", catArgs...) + cat.Stdout = pw + cat.Stderr = os.Stderr + + if err := cat.Start(); err != nil { + _ = pr.Close() + _ = pw.Close() + return nil, nil, errors.New("error occurred during copying dump file: " + src + ", please try again.") + } + + wait := func() error { + return cat.Wait() + } + + go func() { + _ = pw.CloseWithError(wait()) + }() + + return pr, wait, nil +} + // CopyOverCatGunzip streams a remote gzip-compressed file via cf ssh and decompresses // it on the fly, saving the result at dest. func CopyOverCatGunzip(args []string, src string, dest string) error { @@ -267,7 +302,7 @@ func CopyOverCatGunzip(args []string, src string, dest string) error { }() pr, pw := io.Pipe() - catArgs := append(args, "cat \""+src+"\"") //nolint:gocritic // intentional new slice + catArgs := append(args, remoteCatCommand(src)) //nolint:gocritic // intentional new slice cat := exec.Command("cf", catArgs...) cat.Stdout = pw From b92623b03e27bcdfc0d371ebcfde9c6d68a44b9b Mon Sep 17 00:00:00 2001 From: Johannes Bechberger Date: Mon, 21 Sep 2026 10:36:06 +0200 Subject: [PATCH 30/39] Fix changelog heading duplication --- CHANGELOG.md | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7152d3d..07c956f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ### Changed - macOS plugin support now requires Apple Silicon. macOS Intel (`darwin/amd64`) is not supported. +- Improved SSH error messages for better clarity and debugging ### Added @@ -32,10 +33,6 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - `heap-dump --open-url `: override the hprof-analyzer base URL (e.g. a locally running instance). Implies `--open`. -### Changed - -- Improved SSH error messages for better clarity and debugging - ## [4.0.2] ### Fixed From f1667666f744ace6fb9f6d5229fd452b994f018c Mon Sep 17 00:00:00 2001 From: Johannes Bechberger Date: Mon, 21 Sep 2026 10:45:09 +0200 Subject: [PATCH 31/39] Improve heap-dump stream error handling --- cf_cli_java_plugin.go | 15 +++------------ redact.go | 24 ++++++++++++++++++++++++ redact_test.go | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 63 insertions(+), 12 deletions(-) diff --git a/cf_cli_java_plugin.go b/cf_cli_java_plugin.go index ca6a160..fa3a611 100644 --- a/cf_cli_java_plugin.go +++ b/cf_cli_java_plugin.go @@ -1338,19 +1338,10 @@ func (c *JavaPlugin) execute(_ plugin.CliConnection, args []string) (string, err closeErr := reader.Close() waitErr := waitRemote() if rerr != nil { - if closeErr != nil { - c.logVerbosef("warning: closing redaction input stream failed: %v", closeErr) - } - if waitErr != nil { - c.logVerbosef("warning: remote stream wait after redaction failure failed: %v", waitErr) - } - return "", fmt.Errorf("redaction failed: %w", rerr) - } - if closeErr != nil { - return "", fmt.Errorf("redaction input stream close failed: %w", closeErr) + return "", combineHeapDumpStreamErrors(rerr, closeErr, waitErr) } - if waitErr != nil { - return "", fmt.Errorf("download failed while streaming redaction input: %w", waitErr) + if combinedErr := combineHeapDumpStreamErrors(nil, closeErr, waitErr); combinedErr != nil { + return "", combinedErr } c.logVerbosef("Redacted heap dump stream completed successfully") diff --git a/redact.go b/redact.go index a0a7837..a6f2324 100644 --- a/redact.go +++ b/redact.go @@ -10,6 +10,7 @@ import ( "crypto/sha256" _ "embed" "encoding/hex" + "errors" "fmt" "io" "os" @@ -134,3 +135,26 @@ func pipeHeapDumpThroughRedact(redactBin string, input io.Reader, outputBasePath return outputPath, nil } + +func combineHeapDumpStreamErrors(redactErr, closeErr, waitErr error) error { + parts := make([]string, 0, 3) + joined := make([]error, 0, 3) + + if redactErr != nil { + parts = append(parts, "redaction failed") + joined = append(joined, redactErr) + } + if closeErr != nil { + parts = append(parts, "closing redaction input stream failed") + joined = append(joined, closeErr) + } + if waitErr != nil { + parts = append(parts, "remote heap dump stream failed") + joined = append(joined, waitErr) + } + if len(joined) == 0 { + return nil + } + + return fmt.Errorf("%s: %w", strings.Join(parts, "; "), errors.Join(joined...)) +} diff --git a/redact_test.go b/redact_test.go index c6dd217..f9e3432 100644 --- a/redact_test.go +++ b/redact_test.go @@ -3,9 +3,11 @@ package main import ( "bytes" "compress/gzip" + "errors" "fmt" "os" "path/filepath" + "strings" "testing" ) @@ -138,3 +140,37 @@ func TestPipeHeapDumpThroughRedact_RejectsUnexpectedExtension(t *testing.T) { t.Fatal("expected unsupported extension error, got nil") } } + +func TestCombineHeapDumpStreamErrors(t *testing.T) { + redactErr := errors.New("redact boom") + closeErr := errors.New("close boom") + waitErr := errors.New("wait boom") + + err := combineHeapDumpStreamErrors(redactErr, closeErr, waitErr) + if err == nil { + t.Fatal("expected combined error, got nil") + } + + for _, want := range []string{ + "redaction failed", + "closing redaction input stream failed", + "remote heap dump stream failed", + "redact boom", + "close boom", + "wait boom", + } { + if !strings.Contains(err.Error(), want) { + t.Fatalf("expected combined error to contain %q, got %q", want, err.Error()) + } + } + + if !errors.Is(err, redactErr) || !errors.Is(err, closeErr) || !errors.Is(err, waitErr) { + t.Fatal("expected combined error to match all component errors via errors.Is") + } +} + +func TestCombineHeapDumpStreamErrors_NilWhenNoErrors(t *testing.T) { + if err := combineHeapDumpStreamErrors(nil, nil, nil); err != nil { + t.Fatalf("expected nil, got %v", err) + } +} From 08f904e802a699b6fa9fc51413f08b06a471d0f9 Mon Sep 17 00:00:00 2001 From: Johannes Bechberger Date: Mon, 21 Sep 2026 16:22:56 +0200 Subject: [PATCH 32/39] fix: prevent double-call to cat.Wait() in StreamOverCat MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The goroutine that closes pw already calls wait() (which calls cat.Wait()). The caller in cf_cli_java_plugin.go then calls waitRemote() — the same closure — a second time. The second cat.Wait() returns an error because the process was already reaped. Fix: cache the result with sync.Once so wait() is idempotent. Both the goroutine and the caller see the correct exit error regardless of order. --- utils/cfutils.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/utils/cfutils.go b/utils/cfutils.go index cfddef1..3bdb8fe 100644 --- a/utils/cfutils.go +++ b/utils/cfutils.go @@ -15,6 +15,7 @@ import ( "slices" "sort" "strings" + "sync" "github.com/lithammer/fuzzysearch/fuzzy" ) @@ -272,8 +273,13 @@ func StreamOverCat(args []string, src string) (io.ReadCloser, func() error, erro return nil, nil, errors.New("error occurred during copying dump file: " + src + ", please try again.") } + var ( + waitOnce sync.Once + waitResult error + ) wait := func() error { - return cat.Wait() + waitOnce.Do(func() { waitResult = cat.Wait() }) + return waitResult } go func() { From 2d4dddac581ae0d535eac9688d833a4e8756f995 Mon Sep 17 00:00:00 2001 From: Johannes Bechberger Date: Mon, 21 Sep 2026 17:27:10 +0200 Subject: [PATCH 33/39] fix: bump vulnerable deps and extend PR validation CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit deps: - golang.org/x/crypto v0.52.0 → v0.56.0 (fixes GO-2026-6303/6354/6355) - golang.org/x/text v0.37.0 → v0.41.0 (fixes GO-2026-5970) - golang.org/x/sys/term bumped transitively ci (pr-validation.yml): - add explicit `go test -v -race ./...` step to the validate job - split off a `build` job (matrix: ubuntu/macos/windows) that runs after validate passes; each OS runs `go test -race`, builds the plugin with build.py, and uploads artifacts so colleagues can download and test the binaries directly from the Actions run page --- .github/workflows/pr-validation.yml | 102 +++++++++++++++------------- go.mod | 10 +-- go.sum | 10 +++ 3 files changed, 68 insertions(+), 54 deletions(-) diff --git a/.github/workflows/pr-validation.yml b/.github/workflows/pr-validation.yml index 7d6ad20..3f352e1 100644 --- a/.github/workflows/pr-validation.yml +++ b/.github/workflows/pr-validation.yml @@ -10,8 +10,8 @@ permissions: contents: read jobs: - validate-pr: - name: Validate Pull Request + validate: + name: Lint & Test runs-on: ubuntu-latest steps: @@ -52,9 +52,6 @@ jobs: - name: Run govulncheck run: | go install golang.org/x/vuln/cmd/govulncheck@latest - # Run in JSON mode and emit GitHub Actions warning annotations for each finding. - # govulncheck outputs multi-line JSON objects (not line-delimited), so we use - # raw_decode to parse successive top-level objects from the output stream. govulncheck -json . 2>/dev/null | python3 -c " import sys, json decoder = json.JSONDecoder() @@ -90,15 +87,16 @@ jobs: - name: Lint Go code run: ./scripts/lint-go.sh ci + - name: Run Go tests + run: go test -v -race ./... + - name: Check Python test suite id: check-python run: | if [ -f "test/requirements.txt" ] && [ -f "test/setup.sh" ]; then echo "python_tests_exist=true" >> $GITHUB_OUTPUT - echo "✅ Python test suite found" else echo "python_tests_exist=false" >> $GITHUB_OUTPUT - echo "⚠️ Python test suite not found - skipping Python validation" fi - name: Setup Python environment @@ -117,48 +115,54 @@ jobs: - name: Lint Markdown files run: ./scripts/lint-markdown.sh ci - # TODO: Re-enable Python tests when ready - # - name: Run Python tests - # if: steps.check-python.outputs.python_tests_exist == 'true' - # run: | - # cd test - # source venv/bin/activate - # echo "🧪 Running Python tests..." - # if ! pytest -v --tb=short; then - # echo "❌ Python tests failed." - # exit 1 - # fi - # echo "✅ Python tests passed!" - # env: - # CF_API: ${{ secrets.CF_API }} - # CF_USERNAME: ${{ secrets.CF_USERNAME }} - # CF_PASSWORD: ${{ secrets.CF_PASSWORD }} - # CF_ORG: ${{ secrets.CF_ORG }} - # CF_SPACE: ${{ secrets.CF_SPACE }} + build: + name: Build (${{ matrix.os }}) + needs: validate + runs-on: ${{ matrix.os }} + strategy: + matrix: + os: [ubuntu-latest, macos-latest, windows-latest] - - name: Build plugin - run: | - echo "🔨 Building plugin..." - if ! python3 .github/workflows/build.py; then - echo "❌ Build failed." - exit 1 - fi - echo "✅ Build successful!" + steps: + - name: Checkout code + uses: actions/checkout@v6 + + - name: Set up Go + uses: actions/setup-go@v6 + with: + go-version: ">=1.23.5" + + - name: Set up Python + uses: actions/setup-python@v6 + with: + python-version: "3.11" - - name: Validation Summary + - name: Download JStall minimal JAR for go:embed + shell: bash run: | - echo "" - echo "🎉 Pull Request Validation Summary" - echo "==================================" - echo "✅ Go code formatting and linting" - echo "✅ Go tests" - echo "✅ Markdown formatting and linting" - if [ "${{ steps.check-python.outputs.python_tests_exist }}" == "true" ]; then - echo "✅ Python code quality checks" - echo "✅ Python tests" - else - echo "⚠️ Python tests skipped (not found)" - fi - echo "✅ Plugin build" - echo "" - echo "🚀 Ready for merge!" + mkdir -p dist + curl -fsSL -o dist/jstall-minimal.jar https://github.com/parttimenerd/jstall/releases/latest/download/jstall-minimal.jar + + - name: Download hprof-redact binaries for go:embed + shell: bash + run: python3 .github/workflows/build.py --deps-only + + - name: Install Go dependencies + run: go mod tidy -e || true + + - name: Run Go tests + shell: bash + run: go test -race ./... + + - name: Build plugin + shell: bash + run: python3 .github/workflows/build.py + + - name: Upload artifacts + uses: actions/upload-artifact@v4 + with: + name: cf-cli-java-plugin-${{ matrix.os }} + path: | + dist/* + !dist/jstall-minimal.jar + !dist/hprof-redact-* diff --git a/go.mod b/go.mod index ebd3fee..ed77c4b 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module cf.plugin.ref/requires -go 1.25.0 +go 1.26.0 require ( code.cloudfoundry.org/cli v0.0.0-20250623142502-fb19e7a825ee @@ -33,10 +33,10 @@ require ( github.com/sirupsen/logrus v1.9.3 // indirect github.com/stretchr/testify v1.10.0 // indirect github.com/vito/go-interact v0.0.0-20171111012221-fa338ed9e9ec // indirect - golang.org/x/crypto v0.52.0 // indirect - golang.org/x/sys v0.45.0 // indirect - golang.org/x/term v0.43.0 // indirect - golang.org/x/text v0.37.0 // indirect + golang.org/x/crypto v0.56.0 // indirect + golang.org/x/sys v0.47.0 // indirect + golang.org/x/term v0.45.0 // indirect + golang.org/x/text v0.41.0 // indirect gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect gopkg.in/cheggaaa/pb.v1 v1.0.28 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect diff --git a/go.sum b/go.sum index 8a0da8a..23378ac 100644 --- a/go.sum +++ b/go.sum @@ -192,6 +192,8 @@ golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPh golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= golang.org/x/crypto v0.52.0 h1:RMs7fP2rXdep0CftQlK8Uf+kibLm7qkCcradZWYz988= golang.org/x/crypto v0.52.0/go.mod h1:1QgfPxDqh0T2M/elOJtp9RvuR95kVjir0e6/BvEmGbc= +golang.org/x/crypto v0.56.0 h1:GUh5Ii4J5jtcseSMiRqr1jXCNHoxjeV9Fmekc2oLy6Y= +golang.org/x/crypto v0.56.0/go.mod h1:OMW5y6CY9l38uPLmxU6l6pwcXp1obtLo3e6gT7gQR2I= golang.org/x/exp/typeparams v0.0.0-20220218215828-6cf2b201936e/go.mod h1:AbB0pIl9nAr9wVwH+Z2ZpaocVmF5I4GyWCDIsVjR0bk= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.6.0-dev.0.20220106191415-9b9b3d81d5e3/go.mod h1:3p9vT2HGsQu2K1YbXdKPJLVgG5VJdoTa1poYQBtP1AY= @@ -211,6 +213,7 @@ golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug golang.org/x/net v0.6.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= golang.org/x/net v0.54.0 h1:2zJIZAxAHV/OHCDTCOHAYehQzLfSXuf/5SoL/Dv6w/w= golang.org/x/net v0.54.0/go.mod h1:Sj4oj8jK6XmHpBZU/zWHw3BV3abl4Kvi+Ut7cQcY+cQ= +golang.org/x/net v0.57.0 h1:K5+3DljvIuDG9/Jv9rvyMywYNFCQ9RSUY6OOTTkT+tE= golang.org/x/oauth2 v0.27.0 h1:da9Vo7/tDv5RH/7nZDz1eMGS/q1Vv1N/7FCrBhI9I3M= golang.org/x/oauth2 v0.27.0/go.mod h1:onh5ek6nERTohokkhCD/y2cV4Do3fxFHFuAejCkRWT8= golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= @@ -243,11 +246,15 @@ golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.45.0 h1:dO4czNzziLiiXplLQgBCEpCvXQ3dnkn0SdaZSYdQ+FY= golang.org/x/sys v0.45.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/sys v0.47.0 h1:o7XGOvZQCADBQQ4Y7VNq2dRWQR7JmOUW8Kxx4ZsNgWs= +golang.org/x/sys v0.47.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k= golang.org/x/term v0.43.0 h1:S4RLU2sB31O/NCl+zFN9Aru9A/Cq2aqKpTZJ6B+DwT4= golang.org/x/term v0.43.0/go.mod h1:lrhlHNdQJHO+1qVYiHfFKVuVioJIheAc3fBSMFYEIsk= +golang.org/x/term v0.45.0 h1:NwWyBmoJCbfTHpxrWoZ9C6/VxOf7ic219I8xZZFdrf0= +golang.org/x/term v0.45.0/go.mod h1:9aqxs0blBcrm/n0L9QW0aRVD+ktan8ssZromtqJC43w= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= @@ -256,6 +263,8 @@ golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= golang.org/x/text v0.9.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8= golang.org/x/text v0.37.0 h1:Cqjiwd9eSg8e0QAkyCaQTNHFIIzWtidPahFWR83rTrc= golang.org/x/text v0.37.0/go.mod h1:a5sjxXGs9hsn/AJVwuElvCAo9v8QYLzvavO5z2PiM38= +golang.org/x/text v0.41.0 h1:vz/seA0lnX87Othu2f/0L24RcgrXD9/YFTSuGjj3rH8= +golang.org/x/text v0.41.0/go.mod h1:jvf1O8ajNzZqhSrQBPbutR/EB83Cc0CFrezNQIwbb5M= golang.org/x/time v0.9.0 h1:EsRrnYcQiGH+5FfbgvV4AP7qEZstoyrHB0DzarOQ4ZY= golang.org/x/time v0.9.0/go.mod h1:3BpzKBy/shNhVucY/MWOyx10tF3SFh9QdLuxbVysPQM= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= @@ -267,6 +276,7 @@ golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU= golang.org/x/tools v0.44.0 h1:UP4ajHPIcuMjT1GqzDWRlalUEoY+uzoZKnhOjbIPD2c= golang.org/x/tools v0.44.0/go.mod h1:KA0AfVErSdxRZIsOVipbv3rQhVXTnlU6UhKxHd1seDI= +golang.org/x/tools v0.48.0 h1:3+hClM1aLL5mjMKm5ovokw9epgRXPuu2tILgismM6RE= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= From 51ff39cbdac52bac18ca0f5b769d7e49af84e899 Mon Sep 17 00:00:00 2001 From: Johannes Bechberger Date: Mon, 21 Sep 2026 17:43:24 +0200 Subject: [PATCH 34/39] fix: make redact tests Windows-compatible; close file handle before Remove - Use .bat scripts on Windows in test helpers (makeFakeBin, makeCopyBin, makeWriteAndFailBin) so test binaries are executable without needing shell support; use osWindows constant instead of string literal - Close *os.File before os.Remove in pipeHeapDumpThroughRedact pre-check to avoid Windows file-locking errors ("being used by another process") - Strip trailing \r\n in HappyPath test to account for Windows `more` appending CRLF --- .tool.yaml | 4 +- redact.go | 4 +- redact_test.go | 112 +++++++++++++++++++++++++++++++------------------ 3 files changed, 76 insertions(+), 44 deletions(-) diff --git a/.tool.yaml b/.tool.yaml index a52323e..234c538 100644 --- a/.tool.yaml +++ b/.tool.yaml @@ -119,7 +119,7 @@ how_to: cf java heap-dump $APP_NAME --open # Spins up a local server and opens hprof-analyzer in the browser automatically. # On macOS with the Application Firewall enabled, click Allow when prompted. - # In Firefox, allow the page to access local services when prompted. + # In the browsers, allow the page to access local services when prompted. ``` To remove sensitive data (passwords, tokens) before saving: ```bash @@ -244,7 +244,7 @@ how_to: **macOS:** if the Application Firewall is enabled, click **Allow** when asked whether `cf-cli-java-plugin` may accept incoming network connections. - **Firefox:** click **Allow** when the browser asks for permission to access local services. + **Browsers:** click **Allow** when the browser asks for permission to access local services. To use a locally running hprof-analyzer instance instead of the hosted one: ```bash diff --git a/redact.go b/redact.go index a6f2324..eeba3aa 100644 --- a/redact.go +++ b/redact.go @@ -106,9 +106,11 @@ func pipeHeapDumpThroughRedact(redactBin string, input io.Reader, outputBasePath return "", fmt.Errorf("cannot create local directory %s: %w", filepath.Dir(outputPath), err) } - if _, err := os.OpenFile(outputPath, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o600); err != nil { //nolint:gosec // plugin-constructed path + // Pre-check write access; close immediately so Windows doesn't hold a lock. + if f, err := os.OpenFile(outputPath, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o600); err != nil { //nolint:gosec // plugin-constructed path return "", fmt.Errorf("error creating local file at %s: %w", outputPath, err) } else { + _ = f.Close() _ = os.Remove(outputPath) } diff --git a/redact_test.go b/redact_test.go index f9e3432..31c9fac 100644 --- a/redact_test.go +++ b/redact_test.go @@ -7,14 +7,24 @@ import ( "fmt" "os" "path/filepath" + "runtime" "strings" "testing" ) -// makeFakeBin writes a shell script that exits with the given code and returns its path. +// makeFakeBin writes a script that exits with the given code and returns its path. +// On Windows it writes a .bat file; on Unix a shell script. func makeFakeBin(t *testing.T, exitCode int) string { t.Helper() tmp := t.TempDir() + if runtime.GOOS == osWindows { + bin := filepath.Join(tmp, "fake-redact.bat") + script := fmt.Sprintf("@echo off\r\nexit /b %d\r\n", exitCode) + if err := os.WriteFile(bin, []byte(script), 0o600); err != nil { + t.Fatalf("write fake bin: %v", err) + } + return bin + } bin := filepath.Join(tmp, "fake-redact") script := fmt.Sprintf("#!/bin/sh\nexit %d\n", exitCode) if err := os.WriteFile(bin, []byte(script), 0o755); err != nil { //nolint:gosec // test binary must be executable @@ -23,66 +33,90 @@ func makeFakeBin(t *testing.T, exitCode int) string { return bin } -func TestPipeHeapDumpThroughRedact_ErrorDeletesPartial(t *testing.T) { +// makeCopyBin writes a script that copies stdin to $2 (Unix) or %2 (Windows). +func makeCopyBin(t *testing.T) string { + t.Helper() tmp := t.TempDir() - partialBase := filepath.Join(tmp, "dump.hprof") - partial := partialBase - if err := os.WriteFile(partial, []byte("PARTIAL"), 0o600); err != nil { - t.Fatal(err) + if runtime.GOOS == osWindows { + bin := filepath.Join(tmp, "fake-redact.bat") + // On Windows, read stdin and write to the output path argument. + // `more` preserves stdin content; redirect to %2. + script := "@echo off\r\nmore > \"%2\"\r\nexit /b 0\r\n" + if err := os.WriteFile(bin, []byte(script), 0o600); err != nil { + t.Fatalf("write copy bin: %v", err) + } + return bin + } + bin := filepath.Join(tmp, "fake-redact") + script := "#!/bin/sh\ncat - > \"$2\"\n" + if err := os.WriteFile(bin, []byte(script), 0o755); err != nil { //nolint:gosec // test binary must be executable + t.Fatalf("write copy bin: %v", err) } + return bin +} + +// makeWriteAndFailBin writes a script that writes PARTIAL to $2/%2 then exits 1. +func makeWriteAndFailBin(t *testing.T) string { + t.Helper() + tmp := t.TempDir() + if runtime.GOOS == osWindows { + bin := filepath.Join(tmp, "fake-redact.bat") + script := "@echo off\r\necho PARTIAL> \"%2\"\r\nexit /b 1\r\n" + if err := os.WriteFile(bin, []byte(script), 0o600); err != nil { + t.Fatalf("write write-and-fail bin: %v", err) + } + return bin + } + bin := filepath.Join(tmp, "fake-redact") + script := "#!/bin/sh\necho PARTIAL > \"$2\"\nexit 1\n" + if err := os.WriteFile(bin, []byte(script), 0o755); err != nil { //nolint:gosec // test binary must be executable + t.Fatalf("write write-and-fail bin: %v", err) + } + return bin +} + +func TestPipeHeapDumpThroughRedact_ErrorDeletesPartial(t *testing.T) { + tmp := t.TempDir() + outputPath := filepath.Join(tmp, "dump.hprof") failBin := makeFakeBin(t, 1) - _, err := pipeHeapDumpThroughRedact(failBin, bytes.NewBufferString("FAKE"), partialBase, "lean", false) + _, err := pipeHeapDumpThroughRedact(failBin, bytes.NewBufferString("FAKE"), outputPath, "lean", false) if err == nil { t.Fatal("expected error from failing redact binary, got nil") } - if _, statErr := os.Stat(partial); !os.IsNotExist(statErr) { + if _, statErr := os.Stat(outputPath); !os.IsNotExist(statErr) { t.Error("partial redacted file should have been deleted on error, but still exists") } } func TestPipeHeapDumpThroughRedact_KeepOnError(t *testing.T) { tmp := t.TempDir() - base := filepath.Join(tmp, "dump.hprof") - partial := base + outputPath := filepath.Join(tmp, "dump.hprof") - binDir := t.TempDir() - failBin := filepath.Join(binDir, "fake-redact") - script := "#!/bin/sh\necho PARTIAL > \"$2\"\nexit 1\n" - if err := os.WriteFile(failBin, []byte(script), 0o755); err != nil { //nolint:gosec // test binary must be executable - t.Fatal(err) - } - _, err := pipeHeapDumpThroughRedact(failBin, bytes.NewBufferString("FAKE"), base, "lean", true) + bin := makeWriteAndFailBin(t) + _, err := pipeHeapDumpThroughRedact(bin, bytes.NewBufferString("FAKE"), outputPath, "lean", true) if err == nil { t.Fatal("expected error from failing redact binary, got nil") } - if _, statErr := os.Stat(partial); statErr != nil { + if _, statErr := os.Stat(outputPath); statErr != nil { t.Error("partial file should have been kept with keepOnError=true, but is gone") } } func TestPipeHeapDumpThroughRedact_HappyPath(t *testing.T) { tmp := t.TempDir() - base := filepath.Join(tmp, "dump.hprof") + outputPath := filepath.Join(tmp, "dump.hprof") - // Fake binary: copy stdin to output and exit 0 - binDir := t.TempDir() - bin := filepath.Join(binDir, "fake-redact") - script := "#!/bin/sh\ncat - > \"$2\"\n" - if err := os.WriteFile(bin, []byte(script), 0o755); err != nil { //nolint:gosec // test binary must be executable - t.Fatal(err) - } - - out, err := pipeHeapDumpThroughRedact(bin, bytes.NewBufferString("HEAP"), base, "lean", false) + bin := makeCopyBin(t) + out, err := pipeHeapDumpThroughRedact(bin, bytes.NewBufferString("HEAP"), outputPath, "lean", false) if err != nil { t.Fatalf("expected success, got: %v", err) } - want := filepath.Join(tmp, "dump.hprof") - if out != want { - t.Errorf("output path: want %q, got %q", want, out) + if out != outputPath { + t.Errorf("output path: want %q, got %q", outputPath, out) } if _, statErr := os.Stat(out); statErr != nil { t.Errorf("output file missing: %v", statErr) @@ -91,14 +125,16 @@ func TestPipeHeapDumpThroughRedact_HappyPath(t *testing.T) { if readErr != nil { t.Fatalf("read output: %v", readErr) } - if string(data) != "HEAP" { + // Windows `more` appends \r\n; strip for comparison + content := strings.TrimRight(string(data), "\r\n") + if content != "HEAP" { t.Fatalf("unexpected output content: %q", string(data)) } } func TestPipeHeapDumpThroughRedact_PassesCompressedInputUnchanged(t *testing.T) { tmp := t.TempDir() - base := filepath.Join(tmp, "dump.hprof") + outputPath := filepath.Join(tmp, "dump.hprof") var compressed bytes.Buffer gz := gzip.NewWriter(&compressed) @@ -109,14 +145,8 @@ func TestPipeHeapDumpThroughRedact_PassesCompressedInputUnchanged(t *testing.T) t.Fatalf("gzip close: %v", err) } - binDir := t.TempDir() - bin := filepath.Join(binDir, "fake-redact") - script := "#!/bin/sh\ncat - > \"$2\"\n" - if err := os.WriteFile(bin, []byte(script), 0o755); err != nil { //nolint:gosec // test binary must be executable - t.Fatal(err) - } - - out, err := pipeHeapDumpThroughRedact(bin, bytes.NewReader(compressed.Bytes()), base, "lean", false) + bin := makeCopyBin(t) + out, err := pipeHeapDumpThroughRedact(bin, bytes.NewReader(compressed.Bytes()), outputPath, "lean", false) if err != nil { t.Fatalf("expected success, got: %v", err) } From f1e68ab800484cac9c316888e99154356a0ceecc Mon Sep 17 00:00:00 2001 From: Johannes Bechberger Date: Mon, 21 Sep 2026 17:46:51 +0200 Subject: [PATCH 35/39] fix: use PowerShell binary copy on Windows instead of `more` to avoid corrupting gzip data in tests --- redact_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/redact_test.go b/redact_test.go index 31c9fac..e0005c1 100644 --- a/redact_test.go +++ b/redact_test.go @@ -39,9 +39,8 @@ func makeCopyBin(t *testing.T) string { tmp := t.TempDir() if runtime.GOOS == osWindows { bin := filepath.Join(tmp, "fake-redact.bat") - // On Windows, read stdin and write to the output path argument. - // `more` preserves stdin content; redirect to %2. - script := "@echo off\r\nmore > \"%2\"\r\nexit /b 0\r\n" + // Use PowerShell to copy stdin in binary mode; `more` corrupts non-text bytes. + script := "@echo off\r\npowershell -Command \"$in=[System.Console]::OpenStandardInput();$out=[System.IO.File]::OpenWrite('%2');$in.CopyTo($out);$out.Close()\"\r\nexit /b 0\r\n" if err := os.WriteFile(bin, []byte(script), 0o600); err != nil { t.Fatalf("write copy bin: %v", err) } From b93362b40deb401e3cdde2b01695a06316b7f7f9 Mon Sep 17 00:00:00 2001 From: Johannes Bechberger Date: Tue, 22 Sep 2026 09:43:20 +0200 Subject: [PATCH 36/39] fix: replace --ssh-prefix with --cf/--ssh; update jstall to v0.7.1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit jstall never had a --ssh-prefix flag — the code was passing an unknown option that caused "Unknown option: --ssh-prefix" for every status/jstall invocation. Fix: - Use --cf (jstall's built-in CF shortcut) for the normal case - Fall back to --ssh with the full cf-ssh command only when --app-instance-index is needed (jstall's --cf doesn't support instance selection) - Remove the --ssh-prefix argument entirely - Update embedded jstall-minimal.jar from 0.6.0 to 0.7.1 (latest) Also add missing Description fields and fix empty ShortName prefix in generateOptionsMapFromFlags so --compress/--open/--redact show proper help text instead of "-,". --- cf_cli_java_plugin.go | 54 +++++++++++++++++++++++++------------------ jstall.go | 15 +++++------- 2 files changed, 37 insertions(+), 32 deletions(-) diff --git a/cf_cli_java_plugin.go b/cf_cli_java_plugin.go index fa3a611..63c39db 100644 --- a/cf_cli_java_plugin.go +++ b/cf_cli_java_plugin.go @@ -305,34 +305,40 @@ var flagDefinitions = []FlagDefinition{ Type: typeString, }, { - Name: flagRedact, - Usage: "redact heap dump (lean mode: zero primitive arrays only) before saving locally", - Type: typeBool, + Name: flagRedact, + Usage: "redact heap dump (lean mode: zero primitive arrays only) before saving locally", + Description: "redact heap dump before saving locally (lean mode: zero primitive arrays only)", + Type: typeBool, }, { - Name: flagRedactComplete, - Usage: "redact heap dump (complete mode: zero all primitive values) before saving locally", - Type: typeBool, + Name: flagRedactComplete, + Usage: "redact heap dump (complete mode: zero all primitive values) before saving locally", + Description: "redact heap dump before saving locally (complete mode: zero all primitive values)", + Type: typeBool, }, { - Name: flagRedactKeepOnError, - Usage: "keep partially-written redacted file if redaction fails (default: delete it)", - Type: typeBool, + Name: flagRedactKeepOnError, + Usage: "keep partially-written redacted file if redaction fails (default: delete it)", + Description: "keep partially-written redacted file if redaction fails (default: delete it)", + Type: typeBool, }, { - Name: flagCompress, - Usage: "compress heap dump on container using jmap gz=1 (JDK 17+) to reduce transfer size; output is .hprof.gz", - Type: typeBool, + Name: flagCompress, + Usage: "compress heap dump on container using jmap gz=1 (JDK 17+) to reduce transfer size; output is .hprof.gz", + Description: "compress heap dump on the container before downloading (JDK 17+, reduces transfer size); output file will be .hprof.gz", + Type: typeBool, }, { - Name: flagOpen, - Usage: "open the heap dump in the hprof-analyzer web app after downloading", - Type: typeBool, + Name: flagOpen, + Usage: "open the heap dump in the hprof-analyzer web app after downloading", + Description: "open the heap dump in the hprof-analyzer web app after downloading", + Type: typeBool, }, { - Name: flagOpenURL, - Usage: "base URL of the hprof-analyzer instance to open (implies --open)", - Type: typeString, + Name: flagOpenURL, + Usage: "base URL of the hprof-analyzer instance to open (implies --open)", + Description: "base URL of the hprof-analyzer instance to open (implies --open)", + Type: typeString, }, } @@ -436,12 +442,14 @@ func (c *JavaPlugin) generateOptionsMapFromFlags() map[string]string { // Generate options from the centralized flag definitions for _, flagDef := range flagDefinitions { - // Create the prefix for the flag (short name with appropriate formatting) - prefix := "-" + flagDef.ShortName - if flagDef.Name == "app-instance-index" { - prefix += " [index]" + var prefix string + if flagDef.ShortName != "" { + prefix = "-" + flagDef.ShortName + if flagDef.Name == "app-instance-index" { + prefix += " [index]" + } + prefix += ", " } - prefix += ", " // Use the Description field for detailed help text options[flagDef.Name] = utils.WrapTextWithPrefix(flagDef.Description, prefix, 80, 27) diff --git a/jstall.go b/jstall.go index 4bc56be..c92d92f 100644 --- a/jstall.go +++ b/jstall.go @@ -190,17 +190,14 @@ func (c *JavaPlugin) executeJstall(appName string, jstallArgs string, appInstanc args := []string{"-jar", jarPath} - // Build SSH command with PATH setup so jps/jcmd are discoverable on remote container - // SAP Java Buildpack puts JDK tools at deep paths not on $PATH - pathSetup := `JDK_BIN=$(dirname "$(find . -executable -name jps 2>/dev/null | head -1)" 2>/dev/null); if [ -n "$JDK_BIN" ]; then export PATH="$JDK_BIN:$PATH"; fi;` - // Shell-quote appName to prevent command injection via a maliciously named CF app. - sshCmd := "cf ssh " + shellQuote(appName) + // Use --cf for the simple case; --ssh when an instance index is needed + // (jstall's --cf shortcut doesn't support --app-instance-index). if appInstanceIndex != -1 { - sshCmd += " --app-instance-index " + strconv.Itoa(appInstanceIndex) + sshCmd := "cf ssh " + shellQuote(appName) + " --app-instance-index " + strconv.Itoa(appInstanceIndex) + " -c" + args = append(args, "--ssh", sshCmd) + } else { + args = append(args, "--cf", appName) } - sshCmd += " -c" - args = append(args, "--ssh", sshCmd) - args = append(args, "--ssh-prefix", pathSetup) if jstallArgs != "" { splitArgs, err := shlex.Split(jstallArgs) From 4376ff281106ac6b6b8f3075567bb5aa28840ef7 Mon Sep 17 00:00:00 2001 From: Johannes Bechberger Date: Tue, 22 Sep 2026 10:00:04 +0200 Subject: [PATCH 37/39] fix: use --ssh instead of --cf for jstall; fix help text alignment - Replace --cf with --ssh "cf ssh -c" so jstall doesn't internally wrap the command in `sh -c`, which fails on Windows (no sh.exe) - Fix continuation-line indentation in OPTIONS help: compute miscLineIndent so all flags align at the same column regardless of ShortName length --- cf_cli_java_plugin.go | 10 ++++++++-- jstall.go | 12 ++++++------ 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/cf_cli_java_plugin.go b/cf_cli_java_plugin.go index 63c39db..9fb810b 100644 --- a/cf_cli_java_plugin.go +++ b/cf_cli_java_plugin.go @@ -451,8 +451,14 @@ func (c *JavaPlugin) generateOptionsMapFromFlags() map[string]string { prefix += ", " } - // Use the Description field for detailed help text - options[flagDef.Name] = utils.WrapTextWithPrefix(flagDef.Description, prefix, 80, 27) + // Use the Description field for detailed help text. + // miscLineIndent aligns continuation lines: prefix + indent must equal the + // widest prefix used ("-i [index], " = 12 chars, indent 19 → total 31). + indent := 31 - len(prefix) + if indent < 0 { + indent = 0 + } + options[flagDef.Name] = utils.WrapTextWithPrefix(flagDef.Description, prefix, 80, indent) } return options diff --git a/jstall.go b/jstall.go index c92d92f..5ec2981 100644 --- a/jstall.go +++ b/jstall.go @@ -190,14 +190,14 @@ func (c *JavaPlugin) executeJstall(appName string, jstallArgs string, appInstanc args := []string{"-jar", jarPath} - // Use --cf for the simple case; --ssh when an instance index is needed - // (jstall's --cf shortcut doesn't support --app-instance-index). + // Always use --ssh so jstall doesn't try to wrap the command in `sh -c` internally + // (--cf uses a shell wrapper that breaks on Windows). + sshCmd := "cf ssh " + shellQuote(appName) if appInstanceIndex != -1 { - sshCmd := "cf ssh " + shellQuote(appName) + " --app-instance-index " + strconv.Itoa(appInstanceIndex) + " -c" - args = append(args, "--ssh", sshCmd) - } else { - args = append(args, "--cf", appName) + sshCmd += " --app-instance-index " + strconv.Itoa(appInstanceIndex) } + sshCmd += " -c" + args = append(args, "--ssh", sshCmd) if jstallArgs != "" { splitArgs, err := shlex.Split(jstallArgs) From 171bb8aa055a894332badfa547a3d08ce90e023d Mon Sep 17 00:00:00 2001 From: Johannes Bechberger Date: Tue, 22 Sep 2026 13:26:20 +0200 Subject: [PATCH 38/39] ci: trigger CI to pick up jstall v0.7.2 and latest hprof-redact nightly From 27c24bcd9ed7f245fb155801fb5a66be94094326 Mon Sep 17 00:00:00 2001 From: Johannes Bechberger Date: Tue, 22 Sep 2026 14:03:59 +0200 Subject: [PATCH 39/39] fix: use --cf for jstall; remove shellQuote (fixes Windows flag parsing error) jstall v0.7.2 fixed --cf to use ProcessBuilder instead of sh -c, so it now works on Windows. Switch back to --cf for the normal case (cleaner, no quoting issues). Remove shellQuote which is no longer needed. For --app-instance-index, fall back to --ssh without shell quoting since jstall also uses ProcessBuilder for --ssh in v0.7.2. --- jstall.go | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/jstall.go b/jstall.go index 5ec2981..26d77dc 100644 --- a/jstall.go +++ b/jstall.go @@ -170,11 +170,6 @@ func formatCommandForDisplay(command string, args []string) string { return command + " " + strings.Join(displayArgs, " ") } -// shellQuote wraps s in single quotes, escaping any single quotes within. -func shellQuote(s string) string { - return "'" + strings.ReplaceAll(s, "'", "'\\''") + "'" -} - func (c *JavaPlugin) executeJstall(appName string, jstallArgs string, appInstanceIndex int, dryRun bool) (string, error) { javaPath, err := findJava17Plus() if err != nil { @@ -190,14 +185,15 @@ func (c *JavaPlugin) executeJstall(appName string, jstallArgs string, appInstanc args := []string{"-jar", jarPath} - // Always use --ssh so jstall doesn't try to wrap the command in `sh -c` internally - // (--cf uses a shell wrapper that breaks on Windows). - sshCmd := "cf ssh " + shellQuote(appName) + // Use --cf which jstall translates to "cf ssh -c" internally via ProcessBuilder + // (no sh -c wrapper since v0.7.2, so this works on Windows too). + // For instance index, fall back to --ssh since --cf doesn't support it. if appInstanceIndex != -1 { - sshCmd += " --app-instance-index " + strconv.Itoa(appInstanceIndex) + sshCmd := "cf ssh " + appName + " --app-instance-index " + strconv.Itoa(appInstanceIndex) + " -c" + args = append(args, "--ssh", sshCmd) + } else { + args = append(args, "--cf", appName) } - sshCmd += " -c" - args = append(args, "--ssh", sshCmd) if jstallArgs != "" { splitArgs, err := shlex.Split(jstallArgs)