feat(cli): Add skills#175
Merged
Merged
Conversation
misonijnik
force-pushed
the
misonijnik/skills
branch
6 times, most recently
from
a21f81e to
3b8d3be
Compare
misonijnik
force-pushed
the
misonijnik/skills
branch
from
f31b646 to
7a4cc52
Compare
The bare `opentaint` binary failed to auto-load the bundled analyzer, autobuilder and rules, falling back to ~/.opentaint/install/lib instead. GetBundledLibPath/GetBundledJREPath resolved artifacts at <exe-dir>/lib, which only matches the flat managed layout (~/.opentaint/install/). The `make install` FHS layout puts the binary in <prefix>/bin and artifacts in <prefix>/lib (siblings), so the bundled tier never matched. Only the opentaint-dev wrapper worked, via its explicit --analyzer-jar ../lib flag. resolveBundledDir now checks both <exe-dir>/<name> (flat) and <exe-dir>/../<name> (FHS), preferring whichever exists. Also bundle the ruleset in `make install` (rules/ruleset -> $(LIBDIR)/rules) so all three artifacts ship in the install lib and load relative to the binary.
When scanning with a make-install (FHS) build, the header printed `Bundled ruleset: rules/v0.2.0` — a nominal version that looked like the stock release even though the bundled lib holds the user's own modified rules. Same for the analyzer/autobuilder jars. ArtifactDisplayVersion is now tier-aware: an artifact resolved from the bundled tier (the lib next to the binary, which the user controls) renders as `custom (<path>)`, since its nominal version may not match its content. Managed install/cache releases still display their version string. Adds resolveArtifactTier (resolveArtifactPath now delegates to it) and a bundled-tier display test case.
- Add exported utils.PathExists; route health and testutil through it, removing duplicate pathExistsCmd/fileExists copies - Rename health.displayVersion to shortVersion to avoid clashing with utils.displayVersion - Add utils.WriteFiles and use it from testrule/testapprox Scaffold, dropping the repeated mkdir+write loops - Document the shared-global coupling between test rule reachability and the scan command
ArtifactDisplayVersion embeds the jar path in the custom label ("custom
(<path>)"), which health then repeated on its own path node. Split the
display helpers by context: ArtifactVersionWithPath keeps the path
inline
for single-line displays (scan, compile), while the new ArtifactVersion
collapses a custom build to a bare "custom" for displays that show the
path separately (health). The custom-vs-release decision is now a shared
isCustomArtifact predicate.
The colon dangled awkwardly on a parent node whose value hangs on the line below. Remove it from the shared FieldNode primitive (and health's label), so scan, compile, and health tree nodes read "Autobuilder" rather than "Autobuilder:". The inline Field "key: value" form keeps its colon.
Collapse logic that had been copy-pasted across the scan and test commands into reusable libs, keeping the command layer thin: - utils.EnsureRulesPath centralizes the built-in rules resolve+download dance (was duplicated in scan, health, test rule run) - addDataflowApproximations / addPassthroughApproximations share the approximation-input loops between scan and test rule run - utils.CopyFile replaces an ad-hoc file copy in test init - testutil.ResolveJar moves the 4-tier test-util JAR lookup out of the cmd layer into its owning lib Also tidy up a few supportability issues surfaced in review: - drop the never-read analyzerError.message field and fold the duplicated print+return in classifyAnalyzerError into one path - gate scan's viper "scan.*" bindings behind a bindViper flag so the reachability alias no longer re-binds them and steal config precedence - normalize a stray 0755 to 0o755 Behavior-preserving except health now reports a rules-resolution error to stderr instead of swallowing it. Build, vet, and tests pass.
Surface the npm package and install-free npx workflow alongside the existing Homebrew and install-script methods, and note install-free npx usage in the usage guide.
A fragment-less join ref names a rule in the referencing file; the analyzer resolves it to "<currentFile>:<shortId>" and filters loaded rules by exact full-id match. Passing the bare id through left same-file referenced rules (three exist in the builtin ruleset) silently dropped from reachability runs.
…classifier
The analyzer exits 0 even when rule-test samples fail; the verdict lives only
in test-result.json, which the CLI never read — so the documented exit-code
contract ('0 = all tests passed') was not enforced. Test runs now parse the
result file, print a pass/fail summary, and exit 2 on failures.
Also: delete cmd/analyzer_exit.go (a verbatim duplicate of internal/analyzer),
share the exit-codes help text and common flag registrations between the two
run commands, skip the builtin-rules download for approximation tests (the
harness rule is self-contained), and surface non-ENOENT project-model stat
errors instead of swallowing them.
… read override from ArtifactDef Every official install channel (install.sh, brew, npm, release archives) lays lib/ flat next to the binary and resolves at the bundled tier, so classifying TierBundled as 'custom' regressed the SARIF tool version and the info trees to 'custom (<local path>)' for all end users. The release pipeline now stamps lib/release-versions.yaml; a bundled resolution matching the embedded versions.yaml keeps its pinned version, while make-install dev layouts (no marker) still read as custom — the original intent of the tier check. The jarPathOverride parameter on the display helpers duplicated ArtifactDef.Override at every call site; the helpers now read the def.
… analyzer's real runtime Single-component mode printed an empty path and exited 0 on resolution failures, so scripts couldn't detect a missing dependency. The runtime component reported a system Java (or a stale install-tier JRE) that the analyzer runner — which pins the managed Adoptium JRE and never tries system Java — would not use. Both now share the runner's probe via utils.FindCurrentManagedJRE, and the autobuilder/analyzer cases collapse into one resolveJarComponent.
…Java runner policies ensureAnalyzerAvailable/ensureAutobuilderAvailable were near-identical bodies in different command files, reached into across commands; both now delegate to one ensureArtifactJar(def) in artifacts.go driven entirely by the ArtifactDef registry. The analyzer/autobuilder JavaRunner construction chains (copied in scan, compile, and the test runner) collapse into newAnalyzerJavaRunner / newAutobuilderJavaRunner.
…flags beat config/env The scan.* viper keys were bound only to scanCmd's flag instances at init time, so when 'test rule reachability' parsed explicit --timeout/--max-memory /--code-flow-limit values, initConfig's viper.Unmarshal silently overwrote them with config-file or OPENTAINT_SCAN_* env values (the bound scan flags had Changed=false). Config loading now runs from the root PersistentPreRunE and rebinds the keys to whichever command is executing.
…nd builtin download currentScanBuilder promised that new flags propagate to every suggestion but dropped --rule-id, --severity, both approximation flags, and --track-external-methods, so the Docker-fallback and --project-model suggestions ran materially different scans. scanPlan.mode fully derived from needsCompilation (ScanMode removed); the builtin-ruleset download loop ran EnsureRulesPath once per builtin entry for nothing.
…ject The Gradle layout lived in the cobra command file and hardcoded 'libs/opentaint-sast-test-util.jar' in the template while the jar copy used testutil.JarName three hops away — a coherent rename would scaffold projects that fail only at sample-compile time. The layout now lives in one package sharing the JarName constant, writes through utils.WriteFiles/CopyFile (no redundant MkdirAll loop), and the jar resolves once per command instead of once per kind. generate_jar.go reuses utils.CopyFile (gaining close-error checking); package-internal identifiers are unexported.
…pper portability - bare 'make cli' (and parallel 'make -j all') died on fresh checkouts: go generate hard-requires the test-util jar but the cli target had no core dependency - a relative PREFIX split the install: root resolved BINDIR against the repo root while cli/Makefile's abspath resolved it against cli/ — BINDIR is now passed absolutized - the opentaint-dev wrapper depended on non-POSIX realpath; it now falls back to plain $0 resolution - 'make install' built the CLI twice (cli prerequisite + go install); cli install now reuses the build output, which also makes BINARY_NAME effective - the core target ran three Gradle invocations; one invocation configures once and parallelizes the jar tasks
Released binaries could never resolve opentaint-sast-test-util.jar: the lib-assembly step shipped only analyzer/autobuilder/rules and goreleaser ran without go generate, so the go:embed held only the README placeholder and 'opentaint test rule|approximation init' hard-failed for every non-source install. The jar is now built and embedded before goreleaser (the Docker build uses the same workspace, so its context picks the generated jar up too) and bundled into lib/. lib/release-versions.yaml marks the assembled lib dir as an unmodified official release so bundled-tier installs keep their pinned version labels.
…l results, and locale encoding A finding file whose sarif_hashes list was rewritten in YAML block style made HASHES_RE.sub a silent no-op: merged hashes were never persisted, so every rescan counted the same hashes as new and clobbered the analyst's verdict back to pending, forever. Both styles now parse and rewrite, and a missing key is prepended rather than dropped. Also: 'results': null (aborted runs) no longer raises TypeError, and all file I/O pins encoding=utf-8.
The 'test rule run' example omitted the test project's test-rules marker ruleset, so the documented flow reported every positive sample as a false negative; the reachability example replaced the builtin default (pflag array semantics), silently breaking ref expansion into builtin lib rules — the debug-rule skill already showed the correct two-ruleset form. The reachability help example also lacked the 'java/' ruleset-root prefix real rule ids carry.
…t paths across skills - the create-pass-through load-check told subagents to run a full scan on the main project model, contradicting appsec-agent's no-scan rule for subagents; it is now explicitly standalone-only - appsec-agent's blanket 'approximation over a built-in errors at load' applied only to dataflow approximations and contradicted its own passThrough merge rule three lines up - analyze-external-methods' tracking templates omitted the artifact field that appsec-agent's single-source-of-truth schema declares - triage.md's verbatim command mixed a skill-dir-relative script path with project-relative arguments; no cwd satisfied both
misonijnik
force-pushed
the
misonijnik/skills
branch
from
7a4cc52 to
627382b
Compare
Remove the comments introduced on this branch (Go, helper scripts, Makefile, release CI workflow). Preserves go: directives, shebangs, Python docstrings, and all pre-existing comments.
Use a single VersionMarkerName constant for both the bundled tier (next to the binary) and the install tier, renaming the bundled marker from release-versions.yaml to .versions to match the install tier. Add an explicit lib/.versions entry to the goreleaser archives so the dotfile is reliably bundled regardless of how lib/** treats hidden files.
The explicit `- src: lib/.versions` archive entries added in b6cb1d7 were redundant and fragile: goreleaser's fileglob already bundles the dotfile via `lib/**`, while a literal pattern with no wildcard hard-fails the entire release when the file is absent ("globbing failed for pattern lib/.versions"). Remove both entries and guard marker creation directly in the workflow so a genuinely missing .versions fails loudly at the point it should be written rather than silently shipping or dying in goreleaser.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.