Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions internal/guard/judge/llama_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,13 +113,15 @@ func TestStartLlamaServerEarlyExitDoesNotWaitForStopTimeout(t *testing.T) {
BinaryPath: binaryPath,
ModelPath: modelPath,
Port: freeTCPPort(t),
StartupTimeout: 2 * time.Second,
// Keep this comfortably above our assertion so the test proves we don't
// wait for the startup timeout when the process exits immediately.
StartupTimeout: 10 * time.Second,
})
if err == nil {
t.Fatal("StartLlamaServer() error = nil, want early exit error")
}
if elapsed := time.Since(start); elapsed > time.Second {
t.Fatalf("early exit took %s, want less than 1s", elapsed)
if elapsed := time.Since(start); elapsed > 3*time.Second {
t.Fatalf("early exit took %s, want less than 3s", elapsed)
}
}

Expand Down
23 changes: 15 additions & 8 deletions internal/localruntime/conversions.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,21 @@ func EvaluateResultFromResult(result hook.Result) EvaluateResult {
func ResultFromEvaluateResult(result EvaluateResult) hook.Result {
decision, ok := hook.NormalizeDecision(result.Decision)
if !ok {
decision = resultFromBool(result.Allowed).Decision
return hook.Result{
Decision: hook.DecisionDeny,
Reason: fmt.Sprintf("invalid local runtime decision %q", result.Decision),
}
Comment on lines 103 to +108
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Preserve legacy allowed fallback

EvaluateResult.Decision is still omitempty on the wire, and both the CLI hook path and local runtime client pass socket responses through this converter. When an older or hand-written local runtime sends a valid legacy response like {"allowed":true} without decision, this branch now returns a deny instead of preserving the previous allow behavior. That can block operations after only the client side is upgraded.

}
allowedByDecision := decision == hook.DecisionAllow
if result.Allowed != allowedByDecision {
return hook.Result{
Decision: hook.DecisionDeny,
Reason: fmt.Sprintf(
"local runtime decision/allowed mismatch: decision=%q allowed=%t",
result.Decision,
result.Allowed,
),
}
}
return hook.Result{
Decision: decision,
Expand All @@ -115,13 +129,6 @@ func ResultFromEvaluateResult(result EvaluateResult) hook.Result {
}
}

func resultFromBool(allowed bool) hook.Result {
if allowed {
return hook.Result{Decision: hook.DecisionAllow}
}
return hook.Result{Decision: hook.DecisionDeny}
}

func marshalMap(value map[string]any) (json.RawMessage, error) {
if value == nil {
return nil, nil
Expand Down
29 changes: 25 additions & 4 deletions internal/localruntime/conversions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package localruntime

import (
"encoding/json"
"strings"
"testing"

"github.com/kontext-security/kontext-cli/internal/hook"
Expand Down Expand Up @@ -131,7 +132,7 @@ func TestResultFromEvaluateResultNormalizesLegacyDecision(t *testing.T) {

result := ResultFromEvaluateResult(EvaluateResult{
Decision: "DENY",
Allowed: true,
Allowed: false,
Reason: "blocked",
RequestID: "req-123",
})
Expand All @@ -144,7 +145,7 @@ func TestResultFromEvaluateResultNormalizesLegacyDecision(t *testing.T) {
}
}

func TestResultFromEvaluateResultFallsBackToAllowedFlag(t *testing.T) {
func TestResultFromEvaluateResultRejectsInvalidDecision(t *testing.T) {
t.Parallel()

result := ResultFromEvaluateResult(EvaluateResult{
Expand All @@ -153,7 +154,27 @@ func TestResultFromEvaluateResultFallsBackToAllowedFlag(t *testing.T) {
Reason: "legacy allow",
})

if result.Decision != hook.DecisionAllow {
t.Fatalf("decision = %q, want allow", result.Decision)
if result.Decision != hook.DecisionDeny {
t.Fatalf("decision = %q, want deny", result.Decision)
}
if !strings.Contains(result.Reason, "invalid local runtime decision") {
t.Fatalf("reason = %q, want invalid local runtime decision", result.Reason)
}
}

func TestResultFromEvaluateResultRejectsAllowedMismatch(t *testing.T) {
t.Parallel()

result := ResultFromEvaluateResult(EvaluateResult{
Decision: "allow",
Allowed: false,
Reason: "contradiction",
})

if result.Decision != hook.DecisionDeny {
t.Fatalf("decision = %q, want deny", result.Decision)
}
if !strings.Contains(result.Reason, "decision/allowed mismatch") {
t.Fatalf("reason = %q, want decision/allowed mismatch", result.Reason)
}
}
13 changes: 10 additions & 3 deletions internal/localruntime/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package localruntime
import (
"context"
"errors"
"fmt"
"net"
"os"
"time"
Expand Down Expand Up @@ -57,7 +58,9 @@ func NewService(opts Options) (*Service, error) {
func (s *Service) SocketPath() string { return s.socketPath }

func (s *Service) Start(ctx context.Context) error {
os.Remove(s.socketPath)
if err := os.Remove(s.socketPath); err != nil && !errors.Is(err, os.ErrNotExist) {
return fmt.Errorf("remove stale socket: %w", err)
}
ln, err := net.Listen("unix", s.socketPath)
if err != nil {
return err
Expand All @@ -74,9 +77,13 @@ func (s *Service) Stop() {
s.cancel()
}
if s.listener != nil {
s.listener.Close()
if err := s.listener.Close(); err != nil {
s.diagnostic.Printf("local runtime close listener: %v\n", err)
}
}
if err := os.Remove(s.socketPath); err != nil && !errors.Is(err, os.ErrNotExist) {
s.diagnostic.Printf("local runtime remove socket: %v\n", err)
}
os.Remove(s.socketPath)
}

func (s *Service) acceptLoop(ctx context.Context) {
Expand Down
34 changes: 27 additions & 7 deletions internal/run/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,16 @@ func Start(ctx context.Context, opts Options) error {
client := backend.NewClient(backend.BaseURL(), tokenManager.Token)

// 3. Create session via ConnectRPC
hostname, _ := os.Hostname()
cwd, _ := os.Getwd()
hostname, err := os.Hostname()
if err != nil {
diagnostics.Printf("start: determine hostname: %v\n", err)
hostname = ""
}
cwd, err := os.Getwd()
if err != nil {
diagnostics.Printf("start: determine cwd: %v\n", err)
cwd = ""
}
createResp, err := client.CreateSession(ctx, &agentv1.CreateSessionRequest{
UserId: identityKey,
Agent: opts.Agent,
Expand All @@ -98,7 +106,9 @@ func Start(ctx context.Context, opts Options) error {
defer func() {
endManagedSession(client, sessionID, os.Stderr)
if sessionDir != "" {
os.RemoveAll(sessionDir)
if err := os.RemoveAll(sessionDir); err != nil {
diagnostics.Printf("start: remove session dir: %v\n", err)
}
}
}()

Expand Down Expand Up @@ -225,7 +235,9 @@ func Start(ctx context.Context, opts Options) error {
return fmt.Errorf("open runtime session: %w", err)
}
defer func() {
_ = sc.RuntimeCore().CloseSession(context.Background(), sessionID)
if err := sc.RuntimeCore().CloseSession(context.Background(), sessionID); err != nil {
diagnostics.Printf("start: close runtime session: %v\n", err)
}
}()

runtimeService, err := localruntime.NewService(localruntime.Options{
Expand All @@ -246,7 +258,10 @@ func Start(ctx context.Context, opts Options) error {
defer runtimeService.Stop()

// 7. Generate hook settings
kontextBin, _ := os.Executable()
kontextBin, err := os.Executable()
if err != nil {
return fmt.Errorf("resolve kontext binary: %w", err)
}
settingsPath, err := GenerateSettings(sessionDir, kontextBin, opts.Agent)
if err != nil {
return fmt.Errorf("generate settings: %w", err)
Expand Down Expand Up @@ -293,7 +308,10 @@ func endManagedSession(client sessionEnder, sessionID string, out io.Writer) {
endCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()

_ = client.EndSession(endCtx, sessionID)
if err := client.EndSession(endCtx, sessionID); err != nil {
fmt.Fprintf(out, "\n! Failed to end session (%s): %v\n", truncateID(sessionID), err)
return
}
Comment on lines +311 to +314
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 security Redact teardown errors

This prints the raw EndSession error directly to stderr, while diagnostic logging goes through the redactor. If the backend or transport error includes a request URL, authorization detail, cookie, or token-shaped parameter, teardown failures can leak that value into terminal or CI logs even when verbose diagnostics are off.

Suggested change
if err := client.EndSession(endCtx, sessionID); err != nil {
fmt.Fprintf(out, "\n! Failed to end session (%s): %v\n", truncateID(sessionID), err)
return
}
if err := client.EndSession(endCtx, sessionID); err != nil {
fmt.Fprintf(out, "\n! Failed to end session (%s): %s\n", truncateID(sessionID), diagnostic.Redact(err.Error()))
return
}

fmt.Fprintf(out, "\n✓ Session ended (%s)\n", truncateID(sessionID))
}

Expand Down Expand Up @@ -1106,7 +1124,9 @@ func launchAgentWithSettings(_ context.Context, agentName, binaryPath string, en
signal.Notify(sigCh, syscall.SIGINT, syscall.SIGTERM)
go func() {
for sig := range sigCh {
_ = cmd.Process.Signal(sig)
if err := cmd.Process.Signal(sig); err != nil && !errors.Is(err, os.ErrProcessDone) {
fmt.Fprintf(os.Stderr, "signal %s: %v\n", agentName, err)
}
}
}()

Expand Down
Loading